Add ability to define DLRN driver in INI config file

[joelcapitao]

We set 'downstream_driver' as default value for 'driver' key.

[jpichon]

Thank you for the patch. Could you clarify a little bit the background for the change, maybe in the commit message?

Additionally, it would be good to add a unit test or two to show that the new driver value gets picked up where expected, and that the changes are transparent/backwards-compatible otherwise. Some of the unit tests data fixtures need to be updated with the new config value, although it would be good to also have a test where it's still missing anyway to prove that the patch_rebaser.py will be backward compatible. Thanks!

(I think some of the Python versions that failed with "Cancelled" are no longer maintained, I'll see about cleaning up the test matrix, although if you happen to have a few spare cycles to do it in a separate PR (updating tox, github actions should cover it I think), that would be super helpful!)

[joelcapitao]

Thank you for the patch. Could you clarify a little bit the background for the change, maybe in the commit message?

Additionally, it would be good to add a unit test or two to show that the new driver value gets picked up where expected, and that the changes are transparent/backwards-compatible otherwise. Some of the unit tests data fixtures need to be updated with the new config value, although it would be good to also have a test where it's still missing anyway to prove that the patch_rebaser.py will be backward compatible. Thanks!

(I think some of the Python versions that failed with "Cancelled" are no longer maintained, I'll see about cleaning up the test matrix, although if you happen to have a few spare cycles to do it in a separate PR (updating tox, github actions should cover it I think), that would be super helpful!)

I updated the check jobs in first commit.
I also updated the unit tests to take into account your comments.

[jpichon]

Sorry for the delay and thank you for the updated commit message. This is really helpful and now I understand better what we are trying to do (so hopefully reviews won't take as long!)

This needs a rebase, as someone fixed the CI issues in all of our repos so the .github/workflows stuff was fixed separately.

I would like to see a couple of new unit tests proving the new behaviour and to help us avoid regressions in the future. I made a couple of suggestions inline and hope the examples are helpful!

[jpichon]

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

[jpichon]

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.