Replaced the fragile/wrong urlopen JSON mock

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.
@@ -1692,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"]
 
@@ -1715,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"]
 
@@ -1741,36 +1741,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