test(gitlab): increase unit test coverage

nejch · JohnVillalovos · commit df072e130aa1 · 2022-05-29T16:56:36.000-07:00
diff --git a/gitlab/client.py b/gitlab/client.py
@@ -208,7 +208,9 @@ def __setstate__(self, state: Dict[str, Any]) -> None:
         self.__dict__.update(state)
         # We only support v4 API at this time
         if self._api_version not in ("4",):
-            raise ModuleNotFoundError(name=f"gitlab.v{self._api_version}.objects")
+            raise ModuleNotFoundError(
+                name=f"gitlab.v{self._api_version}.objects"
+            )  # pragma: no cover, dead code currently
         # NOTE: We must delay import of gitlab.v4.objects until now or
         # otherwise it will cause circular import errors
         import gitlab.v4.objects
diff --git a/gitlab/config.py b/gitlab/config.py
@@ -154,7 +154,7 @@ def _parse_config(self) -> None:
             # CA bundle.
             try:
                 self.ssl_verify = _config.get("global", "ssl_verify")
-            except Exception:
+            except Exception:  # pragma: no cover
                 pass
         except Exception:
             pass
@@ -166,7 +166,7 @@ def _parse_config(self) -> None:
             # CA bundle.
             try:
                 self.ssl_verify = _config.get(self.gitlab_id, "ssl_verify")
-            except Exception:
+            except Exception:  # pragma: no cover
                 pass
         except Exception:
             pass
@@ -197,7 +197,9 @@ def _parse_config(self) -> None:
 
         try:
             self.http_username = _config.get(self.gitlab_id, "http_username")
-            self.http_password = _config.get(self.gitlab_id, "http_password")
+            self.http_password = _config.get(
+                self.gitlab_id, "http_password"
+            )  # pragma: no cover
         except Exception:
             pass
 
diff --git a/tests/functional/cli/test_cli.py b/tests/functional/cli/test_cli.py
@@ -27,6 +27,12 @@ def test_version(script_runner):
     assert ret.stdout.strip() == __version__
 
 
+def test_config_error_with_help_prints_help(script_runner):
+    ret = script_runner.run("gitlab", "-c", "invalid-file", "--help")
+    assert ret.stdout.startswith("usage:")
+    assert ret.returncode == 0
+
+
 @pytest.mark.script_launch_mode("inprocess")
 @responses.activate
 def test_defaults_to_gitlab_com(script_runner, resp_get_project, monkeypatch):
diff --git a/tests/unit/helpers.py b/tests/unit/helpers.py
@@ -4,6 +4,9 @@
 from typing import Optional
 
 import requests
+import responses
+
+MATCH_EMPTY_QUERY_PARAMS = [responses.matchers.query_param_matcher({})]
 
 
 # NOTE: The function `httmock_response` and the class `Headers` is taken from
diff --git a/tests/unit/mixins/test_mixin_methods.py b/tests/unit/mixins/test_mixin_methods.py
@@ -97,8 +97,17 @@ class M(ListMixin, FakeManager):
         pass
 
     url = "http://localhost/api/v4/tests"
+    headers = {
+        "X-Page": "1",
+        "X-Next-Page": "2",
+        "X-Per-Page": "1",
+        "X-Total-Pages": "2",
+        "X-Total": "2",
+        "Link": ("<http://localhost/api/v4/tests" ' rel="next"'),
+    }
     responses.add(
         method=responses.GET,
+        headers=headers,
         url=url,
         json=[{"id": 42, "foo": "bar"}, {"id": 43, "foo": "baz"}],
         status=200,
@@ -109,6 +118,14 @@ class M(ListMixin, FakeManager):
     mgr = M(gl)
     obj_list = mgr.list(iterator=True)
     assert isinstance(obj_list, base.RESTObjectList)
+    assert obj_list.current_page == 1
+    assert obj_list.prev_page is None
+    assert obj_list.next_page == 2
+    assert obj_list.per_page == 1
+    assert obj_list.total == 2
+    assert obj_list.total_pages == 2
+    assert len(obj_list) == 2
+
     for obj in obj_list:
         assert isinstance(obj, FakeObject)
         assert obj.id in (42, 43)
@@ -254,6 +271,25 @@ class M(UpdateMixin, FakeManager):
     assert responses.assert_call_count(url, 1) is True
 
 
+@responses.activate
+def test_update_mixin_uses_post(gl):
+    class M(UpdateMixin, FakeManager):
+        _update_uses_post = True
+
+    url = "http://localhost/api/v4/tests/1"
+    responses.add(
+        method=responses.POST,
+        url=url,
+        json={},
+        status=200,
+        match=[responses.matchers.query_param_matcher({})],
+    )
+
+    mgr = M(gl)
+    mgr.update(1, {})
+    assert responses.assert_call_count(url, 1) is True
+
+
 @responses.activate
 def test_update_mixin_no_id(gl):
     class M(UpdateMixin, FakeManager):
@@ -323,6 +359,25 @@ class TestClass(SaveMixin, base.RESTObject):
     assert responses.assert_call_count(url, 1) is True
 
 
+@responses.activate
+def test_save_mixin_without_new_data(gl):
+    class M(UpdateMixin, FakeManager):
+        pass
+
+    class TestClass(SaveMixin, base.RESTObject):
+        pass
+
+    url = "http://localhost/api/v4/tests/1"
+    responses.add(method=responses.PUT, url=url)
+
+    mgr = M(gl)
+    obj = TestClass(mgr, {"id": 1, "foo": "bar"})
+    obj.save()
+
+    assert obj._attrs["foo"] == "bar"
+    assert responses.assert_call_count(url, 0) is True
+
+
 @responses.activate
 def test_set_mixin(gl):
     class M(SetMixin, FakeManager):
diff --git a/tests/unit/test_base.py b/tests/unit/test_base.py
@@ -78,13 +78,15 @@ class MGR(base.RESTManager):
 
 class TestRESTObject:
     def test_instantiate(self, fake_gitlab, fake_manager):
-        obj = FakeObject(fake_manager, {"foo": "bar"})
+        attrs = {"foo": "bar"}
+        obj = FakeObject(fake_manager, attrs.copy())
 
-        assert {"foo": "bar"} == obj._attrs
+        assert attrs == obj._attrs
         assert {} == obj._updated_attrs
         assert obj._create_managers() is None
         assert fake_manager == obj.manager
         assert fake_gitlab == obj.manager.gitlab
+        assert str(obj) == f"{type(obj)} => {attrs}"
 
     def test_instantiate_non_dict(self, fake_gitlab, fake_manager):
         with pytest.raises(gitlab.exceptions.GitlabParsingError):
@@ -201,6 +203,7 @@ def test_equality(self, fake_manager):
         obj1 = FakeObject(fake_manager, {"id": "foo"})
         obj2 = FakeObject(fake_manager, {"id": "foo", "other_attr": "bar"})
         assert obj1 == obj2
+        assert len(set((obj1, obj2))) == 1
 
     def test_equality_custom_id(self, fake_manager):
         class OtherFakeObject(FakeObject):
@@ -210,6 +213,11 @@ class OtherFakeObject(FakeObject):
         obj2 = OtherFakeObject(fake_manager, {"foo": "bar", "other_attr": "baz"})
         assert obj1 == obj2
 
+    def test_equality_no_id(self, fake_manager):
+        obj1 = FakeObject(fake_manager, {"attr1": "foo"})
+        obj2 = FakeObject(fake_manager, {"attr1": "bar"})
+        assert not obj1 == obj2
+
     def test_inequality(self, fake_manager):
         obj1 = FakeObject(fake_manager, {"id": "foo"})
         obj2 = FakeObject(fake_manager, {"id": "bar"})
@@ -219,6 +227,12 @@ def test_inequality_no_id(self, fake_manager):
         obj1 = FakeObject(fake_manager, {"attr1": "foo"})
         obj2 = FakeObject(fake_manager, {"attr1": "bar"})
         assert obj1 != obj2
+        assert len(set((obj1, obj2))) == 2
+
+    def test_equality_with_other_objects(self, fake_manager):
+        obj1 = FakeObject(fake_manager, {"id": "foo"})
+        obj2 = None
+        assert not obj1 == obj2
 
     def test_dunder_str(self, fake_manager):
         fake_object = FakeObject(fake_manager, {"attr1": "foo"})
@@ -280,3 +294,11 @@ def test_pprint(self, capfd, fake_manager):
             " 'ham': 'eggseggseggseggseggseggseggseggseggseggseggseggseggseggseggs'}\n"
         )
         assert stderr == ""
+
+    def test_repr(self, fake_manager):
+        attrs = {"attr1": "foo"}
+        obj = FakeObject(fake_manager, attrs)
+        assert repr(obj) == "<FakeObject id:None>"
+
+        FakeObject._id_attr = None
+        assert repr(obj) == "<FakeObject>"
diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py
@@ -54,6 +54,15 @@
 oauth_token = STUV
 """
 
+ssl_verify_str_config = """[global]
+default = one
+ssl_verify = /etc/ssl/certs/ca-certificates.crt
+
+[one]
+url = http://one.url
+private_token = ABCDEF
+"""
+
 custom_user_agent_config = f"""[global]
 default = one
 user_agent = {custom_user_agent}
@@ -69,7 +78,7 @@
 private_token = ABCDEF
 """
 
-missing_attr_config = """[global]
+invalid_data_config = """[global]
 [one]
 url = http://one.url
 
@@ -83,6 +92,11 @@
 url = http://four.url
 private_token = ABCDEF
 per_page = 200
+
+[invalid-api-version]
+url = http://invalid-api-version.url
+private_token = ABCDEF
+api_version = 1
 """
 
 
@@ -173,7 +187,7 @@ def test_invalid_id(m_open, mock_clean_env, monkeypatch):
 
 @mock.patch("builtins.open")
 def test_invalid_data(m_open, monkeypatch):
-    fd = io.StringIO(missing_attr_config)
+    fd = io.StringIO(invalid_data_config)
     fd.close = mock.Mock(return_value=None, side_effect=lambda: fd.seek(0))
     m_open.return_value = fd
 
@@ -185,9 +199,14 @@ def test_invalid_data(m_open, monkeypatch):
             config.GitlabConfigParser(gitlab_id="two")
         with pytest.raises(config.GitlabDataError):
             config.GitlabConfigParser(gitlab_id="three")
-        with pytest.raises(config.GitlabDataError) as emgr:
+
+        with pytest.raises(config.GitlabDataError) as e:
             config.GitlabConfigParser("four")
-        assert "Unsupported per_page number: 200" == emgr.value.args[0]
+        assert str(e.value) == "Unsupported per_page number: 200"
+
+        with pytest.raises(config.GitlabDataError) as e:
+            config.GitlabConfigParser("invalid-api-version")
+        assert str(e.value) == "Unsupported API version: 1"
 
 
 @mock.patch("builtins.open")
@@ -248,6 +267,18 @@ def test_valid_data(m_open, monkeypatch):
     assert cp.ssl_verify is True
 
 
+@mock.patch("builtins.open")
+def test_ssl_verify_as_str(m_open, monkeypatch):
+    fd = io.StringIO(ssl_verify_str_config)
+    fd.close = mock.Mock(return_value=None)
+    m_open.return_value = fd
+
+    with monkeypatch.context() as m:
+        m.setattr(Path, "resolve", _mock_existent_file)
+        cp = config.GitlabConfigParser()
+    assert cp.ssl_verify == "/etc/ssl/certs/ca-certificates.crt"
+
+
 @mock.patch("builtins.open")
 @pytest.mark.skipif(sys.platform.startswith("win"), reason="Not supported on Windows")
 def test_data_from_helper(m_open, monkeypatch, tmp_path):
@@ -286,6 +317,33 @@ def test_data_from_helper(m_open, monkeypatch, tmp_path):
     assert "secret" == cp.oauth_token
 
 
+@mock.patch("builtins.open")
+@pytest.mark.skipif(sys.platform.startswith("win"), reason="Not supported on Windows")
+def test_from_helper_subprocess_error_raises_error(m_open, monkeypatch):
+    # using /usr/bin/false here to force a non-zero return code
+    fd = io.StringIO(
+        dedent(
+            """\
+            [global]
+            default = helper
+
+            [helper]
+            url = https://helper.url
+            oauth_token = helper: /usr/bin/false
+            """
+        )
+    )
+
+    fd.close = mock.Mock(return_value=None)
+    m_open.return_value = fd
+    with monkeypatch.context() as m:
+        m.setattr(Path, "resolve", _mock_existent_file)
+        with pytest.raises(config.GitlabConfigHelperError) as e:
+            config.GitlabConfigParser(gitlab_id="helper")
+
+        assert "Failed to read oauth_token value from helper" in str(e.value)
+
+
 @mock.patch("builtins.open")
 @pytest.mark.parametrize(
     "config_string,expected_agent",
diff --git a/tests/unit/test_exceptions.py b/tests/unit/test_exceptions.py
@@ -3,6 +3,18 @@
 from gitlab import exceptions
 
 
+@pytest.mark.parametrize(
+    "kwargs,expected",
+    [
+        ({"error_message": "foo"}, "foo"),
+        ({"error_message": "foo", "response_code": "400"}, "400: foo"),
+    ],
+)
+def test_gitlab_error(kwargs, expected):
+    error = exceptions.GitlabError(**kwargs)
+    assert str(error) == expected
+
+
 def test_error_raises_from_http_error():
     """Methods decorated with @on_http_error should raise from GitlabHttpError."""
 
diff --git a/tests/unit/test_gitlab.py b/tests/unit/test_gitlab.py
diff --git a/tests/unit/test_gitlab_http_methods.py b/tests/unit/test_gitlab_http_methods.py
diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py
diff --git a/tox.ini b/tox.ini
