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

Testing project settings #4

Merged
merged 4 commits into from
Aug 1, 2024
Merged

Conversation

otulach
Copy link
Contributor

@otulach otulach commented Jul 10, 2024

Trying to test more functions. Adjustments to mocks-gitlab-api.

helper=True,
)

def register_project_with_mr(self, numerical_id, full_project_path):
Copy link
Member

Choose a reason for hiding this comment

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

How about keeping just one function and build the JSON from extra arguments.

Something like the following?

def register_project(self, numerical_id, full_project_path, **kwargs):
    response_json_base = {
         'id': numerical_id,
         'path_with_namespace': full_project_path,
    }
    # Not sure about this syntax but Python can merge dictionaries
    response_json = {**response_json_base, **kwargs}
    ...

What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the suggestion.

It sure does look much cleaner. Now I just have to make it work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This new commit should resolve the problem.

However, in this particular case 'mr_default_target_self' isn't involved
in the kwargs, so I had to add this as an base key in the dictionary.

Copy link
Member

Choose a reason for hiding this comment

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

Feel free to drop old code, no need to keep the commented code around if we have Git :-).

However, in this particular case 'mr_default_target_self' isn't involved
in the kwargs, so I had to add this as an base key in the dictionary.

Why? What would prevent you to call register_project(42, '/group/project', mr_default_target_self='self')?

mock_gitlab.get_python_gitlab(),
logging.getLogger("settings"),
tg.ActionEntries(entries),
True,
Copy link
Member

Choose a reason for hiding this comment

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

Here should be False: otherwise the test runs in dry run mode, i.e. it does not perform any modifications on GitLab. I also believe that you should call mock_gitlab.report_unknown() somewhere to ensure that you do not miss any API calls.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the advice, however, I tried doing it the way you said and an issue emerged. Even after the mock_gitlab.report_unknown() I was not able to mock call PUT http://localhost/api/v4/projects/42, which isn't even in the registered responses right?

Copy link
Member

Choose a reason for hiding this comment

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

Right, the first time we need the PUT method ;-).

I think the following should help as it will add on_api_put that behaves similarly to on_api_post and should allow you to mock the PUT call modifying the project settings.

diff --git a/tests/mocks_gitlab_api.py b/tests/mocks_gitlab_api.py
index 2d33c00..e59ef97 100644
--- a/tests/mocks_gitlab_api.py
+++ b/tests/mocks_gitlab_api.py
@@ -22,7 +22,7 @@ class MockedGitLabApi:
             self.unknown_urls.append(log_line)
             return (404, {}, json.dumps({"error": "not implemented"}))
 
-        methods = [responses.GET, responses.POST, responses.DELETE]
+        methods = [responses.GET, responses.POST, responses.DELETE, responses.PUT]
         for m in methods:
             self.responses.add_callback(
                 m,
@@ -97,3 +97,16 @@ class MockedGitLabApi:
             *args,
             **kwargs,
         )
+
+    def on_api_put(self, url, request_json, response_json, *args, **kwargs):
+        kwargs['body'] = json.dumps(response_json)
+        kwargs['match'] = [
+            responses.matchers.json_params_matcher(request_json)
+        ]
+        kwargs['content_type'] = 'application/json'
+
+        return self.responses.put(
+            self.make_api_url_(url),
+            *args,
+            **kwargs,
+        )
diff --git a/tests/test_settings.py b/tests/test_settings.py
index 4f00904..1e416e4 100644
--- a/tests/test_settings.py
+++ b/tests/test_settings.py
@@ -26,12 +26,15 @@ def test_project_settings_with_none(mock_gitlab):
     ]
 
     mock_gitlab.register_project(38, 'student/beta', mr_default_target_self='self')
+    mock_gitlab.on_api_put('projects/42', ...)
+
+    mock_gitlab.report_unknown()
 
     tg.action_project_settings(
         mock_gitlab.get_python_gitlab(),
         logging.getLogger("settings"),
         tg.ActionEntries(entries),
-        True,
+        False,
         'student/{login}',
         None,
         'The best project'

Instead of the three dots in mock_gitlab.on_api_put('projects/42', ...) we need the actual JSON that is sent/returned when updating project settings (at least part of it).

@vhotspur vhotspur merged commit 9abcb94 into d-iii-s:master Aug 1, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants