Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ability to define DLRN driver in INI config file #32

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
python: ['3.7', '3.8', '3.9', '3.10', '3.11']
python: ['3.9', '3.10', '3.11', '3.12']
steps:
- uses: actions/checkout@v3
- uses: actions/checkout@v4
- name: Setup Python
uses: actions/setup-python@v4
with:
Expand All @@ -24,11 +24,11 @@ jobs:
flake8:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- uses: actions/checkout@v4
- name: Setup Python
uses: actions/setup-python@v4
with:
python-version: 3.11
python-version: 3.12
- name: Install Tox and any other packages
run: |
python -m pip install --upgrade pip
Expand All @@ -38,11 +38,11 @@ jobs:
integration:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- uses: actions/checkout@v4
- name: Setup Python
uses: actions/setup-python@v4
with:
python-version: 3.11
python-version: 3.12
- name: Install Tox and any other packages
run: |
python -m pip install --upgrade pip
Expand Down
3 changes: 3 additions & 0 deletions patch_rebaser/patch_rebaser.ini.example
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ git_email = [email protected]
packages_to_process =
# Location of DLRN projects.ini file (optional)
dlrn_projects_ini =
# DLRN driver used, in order to get the parameters from the right
# section of the DLRN projects.ini file. (optional)
dlrn_driver =
# Do not force-push to remotes or do other destructive operations
# while in development mode
dev_mode = true
Expand Down
15 changes: 9 additions & 6 deletions patch_rebaser/patch_rebaser.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,14 +69,14 @@ def create_patches_branch(repo, commit, remote, dev_mode=True):
return branch_name


def get_downstream_distgit_branch(dlrn_projects_ini):
def get_downstream_distgit_branch(dlrn_projects_ini, dlrn_driver):
"""Get downstream distgit branch info from DLRN projects.ini"""
config = configparser.ConfigParser()
config.read(dlrn_projects_ini)
return config.get('downstream_driver', 'downstream_distro_branch')
return config.get(dlrn_driver, 'downstream_distro_branch')


def get_patches_branch(repo, remote, dlrn_projects_ini):
def get_patches_branch(repo, remote, dlrn_projects_ini, dlrn_driver):
"""Get the patches branch name"""
branch_name = os.environ.get('PATCHES_BRANCH', None)
if branch_name:
Expand All @@ -87,7 +87,8 @@ def get_patches_branch(repo, remote, dlrn_projects_ini):
return None
else:
# Get downstream distgit branch from DLRN config
distgit_branch = get_downstream_distgit_branch(dlrn_projects_ini)
distgit_branch = get_downstream_distgit_branch(dlrn_projects_ini,
dlrn_driver)
LOGGER.warning("No PATCHES_BRANCH env var found, trying to guess it")
# Guess at patches branch based on the distgit branch name
return find_patches_branch(repo, remote, distgit_branch)
Expand Down Expand Up @@ -188,7 +189,7 @@ def get_rebaser_config(defaults=None):
"""
default_options = ['dev_mode', 'remote_name', 'git_name', 'git_email',
'packages_to_process', 'dlrn_projects_ini',
'create_patches_branch']
'create_patches_branch', 'dlrn_driver']
distroinfo_options = ['patches_repo_key']

RebaserConfig = namedtuple('RebaserConfig',
Expand Down Expand Up @@ -426,6 +427,7 @@ def main():

# Default values for options in patch_rebaser.ini
defaults = {
'dlrn_driver': 'downstream_driver',
'remote_name': 'remote_name',
'git_name': 'Your Name',
'git_email': '[email protected]',
Expand Down Expand Up @@ -469,7 +471,8 @@ def main():
# Create local patches branch
branch_name = get_patches_branch(repo,
config.remote_name,
config.dlrn_projects_ini)
config.dlrn_projects_ini,
config.dlrn_driver)

# Not every project has a -patches branch for every release
if not branch_name:
Expand Down
8 changes: 6 additions & 2 deletions tests/test_patch_rebaser.py
Original file line number Diff line number Diff line change
Expand Up @@ -306,10 +306,12 @@ def test_get_rebaser_config_with_fallback_value(mock_config):
AND an option doesn't exist in the config
THEN the value from the dictionary is returned
"""
defaults = {'git_name': 'TEST', 'remote_name': 'Wrong_name'}
defaults = {'git_name': 'TEST',
'dlrn_driver': 'downstream_driver'}
config = get_rebaser_config(defaults)

assert config.git_name == defaults['git_name']
assert config.dlrn_driver == defaults['dlrn_driver']


def test_get_rebaser_config_defaults_dont_override_ini_values(mock_config):
Expand All @@ -319,7 +321,9 @@ def test_get_rebaser_config_defaults_dont_override_ini_values(mock_config):
AND an option exists both in the ini config and the defaults dictionary
THEN the value from the ini config is returned
"""
defaults = {'git_name': 'TEST', 'remote_name': 'Wrong_name'}
defaults = {'git_name': 'TEST',
'dlrn_driver': 'downstream_driver',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need this change here since we're only testing a subset of defaults anyway.

'remote_name': 'Wrong_name'}
config = get_rebaser_config(defaults)

assert config.remote_name == "test_remote_name"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GitHub won't let me comment directly below, but you can see test_main_function just under here. I think it could be modified slightly to prove we still get the driver we expect, something like this:

     repo = MagicMock()
+    mock_get_patches_branch = Mock(return_value=branch_name)
 
     with monkeypatch.context() as m:
         m.setattr(patch_rebaser.patch_rebaser, 'GitRepo',
                   Mock(return_value=repo))
         m.setattr(patch_rebaser.patch_rebaser, 'get_patches_repo', Mock())
         m.setattr(patch_rebaser.patch_rebaser, 'get_patches_branch',
-                  Mock(return_value=branch_name))
+                  mock_get_patches_branch)
         main()
 
+    mock_get_patches_branch.assert_called_with(mock.ANY, mock.ANY, mock.ANY, "downstream_driver")

then you could have a separate test to show what happens with the custom configuration, using a new test config file that has a custom dlrn_driver set. This would help us make sure we don't accidentally add regressions related to using custom drivers in the future. conftest.py has a couple of examples of custom test config already. It might look something like this:

def test_main_with_custom_driver(monkeypatch, mock_env, mock_custom_driver_config):
    """
    GIVEN ...
    WHEN ...
    THEN ...
    """
    mock_get_patches_branch = Mock()
    monkeypatch.setattr(patch_rebaser.patch_rebaser, 'get_patches_branch', mock_get_patches_branch)

    # Set up
    monkeypatch.setattr(patch_rebaser.patch_rebaser, 'GitRepo', Mock(return_value=MagicMock()))
    monkeypatch.setattr(patch_rebaser.patch_rebaser, 'get_patches_repo', Mock())
    monkeypatch.setattr(patch_rebaser.patch_rebaser, 'get_release_from_branch_name', Mock())
    monkeypatch.setattr(patch_rebaser.patch_rebaser, 'Rebaser', Mock())

    # Test
    main()

    # Check
    mock_get_patches_branch.assert_called_with(mock.ANY, mock.ANY, mock.ANY, "my_custom_driver")

If you prefer, you could also test for the default driver this way (using mock_config instead of your new mock_custom_driver_config) instead of modifying the other test for the main function.

Expand Down
2 changes: 1 addition & 1 deletion tox.ini
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[tox]
envlist =
py{37,38,39,310,311}
py{39,310,311,312}
flake8

[testenv]
Expand Down
Loading