From f028709a2c0d341ec02b278f49d347c5d201ccc8 Mon Sep 17 00:00:00 2001 From: TheresNoTime Date: Mon, 6 Jul 2026 13:09:33 +0100 Subject: [PATCH 1/2] Replaced the fragile/wrong urlopen JSON mock with a patch of TWLight.users.models.editor_global_userinfo returning FAKE_GLOBAL_USERINFO Assisted-by: Claude Opus 4.8 Bug: T352896 --- TWLight/users/tests.py | 63 +++++++++++++++++++++--------------------- 1 file changed, 31 insertions(+), 32 deletions(-) diff --git a/TWLight/users/tests.py b/TWLight/users/tests.py index 98cb7745c3..3148a74a8b 100644 --- a/TWLight/users/tests.py +++ b/TWLight/users/tests.py @@ -49,8 +49,6 @@ editor_make_block_dict, ) -FAKE_IDENTITY_DATA = {"query": {"userinfo": {"options": {"disablemail": 0}}}} - FAKE_IDENTITY = { "editcount": 5000, "registered": "20151106154629", # Well before first commit. @@ -1741,36 +1739,37 @@ def setUpTestData(cls): for editor in Editor.objects.all(): editor.delete() - @patch("urllib.request.urlopen") - def test_create_user_and_editor(self, mock_urlopen): - """ - OAuthBackend._create_user_and_editor() should: - * create a user - * with a suitable username and email - * without a password - * And a matching editor - """ - # HOTFIX: this test was failing in CI, but passing locally, just skipping it for now - # see: https://phabricator.wikimedia.org/T352896 - self.skipTest("See: https://phabricator.wikimedia.org/T352896") - oauth_backend = OAuthBackend() - oauth_data = FAKE_IDENTITY_DATA - identity = FAKE_IDENTITY - - mock_response = Mock() - mock_response.read.side_effect = [json.dumps(oauth_data)] * 7 - mock_urlopen.return_value = mock_response - - user, editor = oauth_backend._create_user_and_editor(identity) - - self.assertEqual(user.email, "alice@example.com") - self.assertEqual(user.username, "567823") - self.assertFalse(user.has_usable_password()) - - self.assertEqual(editor.user, user) - self.assertEqual(editor.wp_sub, 567823) - # We won't test the fields set by update_from_wikipedia, as they are - # tested elsewhere. + @patch("TWLight.users.models.editor_global_userinfo") + def test_create_user_and_editor(self, mock_global_userinfo): + """ + OAuthBackend._create_user_and_editor() should: + * create a user + * with a suitable username and email + * without a password + * And a matching editor + """ + oauth_backend = OAuthBackend() + identity = copy.copy(FAKE_IDENTITY) + + # _create_editor() calls update_from_wikipedia() without an explicit + # global_userinfo, so it fetches one from the API. Mock that call so the + # editor gets saved (via update_editcount) before its wp_editcount is + # read. Returning None here leaves the editor unsaved and raises + # "Model instances passed to related filters must be saved." (T352896) + global_userinfo = copy.copy(FAKE_GLOBAL_USERINFO) + global_userinfo["id"] = identity["sub"] + mock_global_userinfo.return_value = global_userinfo + + user, editor = oauth_backend._create_user_and_editor(identity) + + self.assertEqual(user.email, "alice@example.com") + self.assertEqual(user.username, "567823") + self.assertFalse(user.has_usable_password()) + + self.assertEqual(editor.user, user) + self.assertEqual(editor.wp_sub, 567823) + # We won't test the fields set by update_from_wikipedia, as they are + # tested elsewhere. # We mock out this function for two reasons: # 1) To prevent its call to an external API, which we would have otherwise From 450740e2f50c9b0bcbbdf97b2c9d14d535a67f59 Mon Sep 17 00:00:00 2001 From: TheresNoTime Date: Mon, 6 Jul 2026 13:21:12 +0100 Subject: [PATCH 2/2] stop mutating the shared fixture --- TWLight/users/tests.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/TWLight/users/tests.py b/TWLight/users/tests.py index 3148a74a8b..46efb32faa 100644 --- a/TWLight/users/tests.py +++ b/TWLight/users/tests.py @@ -1690,8 +1690,9 @@ def test_editor_email_not_null(self): platform, we shouldn't be overwriting it with a blank string. """ new_editor = EditorFactory(wp_registered=None) - new_identity = FAKE_IDENTITY - new_global_userinfo = FAKE_GLOBAL_USERINFO + # Copy so we don't mutate the shared module-level fixtures (T352896). + new_identity = copy.copy(FAKE_IDENTITY) + new_global_userinfo = copy.copy(FAKE_GLOBAL_USERINFO) new_identity["sub"] = new_editor.wp_sub new_global_userinfo["id"] = new_identity["sub"] @@ -1713,8 +1714,9 @@ def test_editor_email_not_overwritten_with_blank(self): platform, we shouldn't be overwriting it with a blank string. """ new_editor = EditorFactory(wp_registered=None) - new_identity = FAKE_IDENTITY - new_global_userinfo = FAKE_GLOBAL_USERINFO + # Copy so we don't mutate the shared module-level fixtures (T352896). + new_identity = copy.copy(FAKE_IDENTITY) + new_global_userinfo = copy.copy(FAKE_GLOBAL_USERINFO) new_identity["sub"] = new_editor.wp_sub new_global_userinfo["id"] = new_identity["sub"]