Merge ~mertkirpici/juju-lint:topic/coverage into juju-lint:master

[Mert Kirpici]

Require full unit test coverage

This was missed during the snap template update, we expect %100 test coverage in this project.

This also revealed a sneaky bug in the process. When the jujulint.config.Config class got initialized it was using the sys.argv of the pytest process, causing a testing error and not being able to pass arguments to pytest.

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

PASSED: Continuous integration, rev:f50f5dd340df2c0f40dc80b129f22e632e12ddc7
https://jenkins.canonical.com/bootstack/job/lp-juju-lint-ci/60/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/2101/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/1939/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-juju-lint-ci/60//rebuild

review: Approve (continuous-integration)

[Robert Gildein (rgildein)]

LGTM

Maybe we can use our templates [1] in juju-lint, not in this MP.

---
[1]: https://git.launchpad.net/bootstack-charms-spec/tree/source/snap-pytest

review: Approve

[Ramesh Sattaru (rameshcan)]

review: Approve

[Mert Kirpici (mertkirpici)]

> LGTM
>
> Maybe we can use our templates [1] in juju-lint, not in this MP.
>
> ---
> [1]: https://git.launchpad.net/bootstack-charms-spec/tree/source/snap-pytest

Hi there Robert, thanks for the review! This snap recently seems to have received the template update but the coverage relation sections in tox and pyproject.toml were missing. I assumed they were also missing in the bootstack templates. Turns out they're not. Maybe they were skipped due to the sys.argv bug I tried to explain in the description. Passing arguments to pytest in tox.ini was broken.

I am thinking of applying both your suggestions and align pyproject.toml and tox.ini posargs in this MP, I think it is a better solution rather than diverging more. What are your thoughts?

[Robert Gildein (rgildein)]

> Hi there Robert, thanks for the review! This snap recently seems to have
> received the template update but the coverage relation sections in tox and
> pyproject.toml were missing. I assumed they were also missing in the bootstack
> templates. Turns out they're not. Maybe they were skipped due to the sys.argv
> bug I tried to explain in the description. Passing arguments to pytest in
> tox.ini was broken.

Maybe it was related with tox 4.* issue? Not sure here.

> I am thinking of applying both your suggestions and align pyproject.toml and
> tox.ini posargs in this MP, I think it is a better solution rather than
> diverging more. What are your thoughts?

It's up to you, I'm trying to suggest templates everywhere, because it's easier
to work on multiple project with similar dev behavior and also using them in various
projects helps to improve them. I think that using separate MP is better, because
you can add/update more templates at once (recently we add .gitignore).

Also feel free to update our templates if it's needed. Recently I removed
`addopts` [1] from `juju-spell`, because it was more annoying than helpful.

---
[1]: https://git.launchpad.net/bootstack-charms-spec/tree/source/snap-pytest/pyproject.toml#n38

them also.

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

PASSED: Continuous integration, rev:fca2a52d8ef38b12e662f7dadd131d9448d0b951
https://jenkins.canonical.com/bootstack/job/lp-juju-lint-ci/61/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/2103/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/1942/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-juju-lint-ci/61//rebuild

review: Approve (continuous-integration)

[Sudeep Bhandari (sudeephb)]

review: Approve

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

Change successfully merged at revision ce5d33a2b39e4a133eb6abbabc5105c1c9e0ea55

diff --git a/pyproject.toml b/pyproject.toml
index c58e8e6..789c704 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -33,3 +33,18 @@ skip_glob = '''
     | report
 )/
 '''
+
+[tool.coverage.run]
+relative_files = true
+source = ["jujulint"]
+omit = ["tests/**", "docs/**", "lib/**", "snap/**", "build/**", "setup.py"]
+
+[tool.coverage.report]
+fail_under = 100
+show_missing = true
+
+[tool.coverage.html]
+directory = "report"
+
+[tool.coverage.xml]
+output = "report/coverage.xml"
diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py
index 1d9060d..71c2e40 100644
--- a/tests/unit/test_config.py
+++ b/tests/unit/test_config.py
@@ -3,12 +3,14 @@
 
 import os
 import sys
+from unittest.mock import patch
 
 import yaml
 
 from jujulint.config import Config
 
-config_file = f"{Config().config_dir()}/config.yaml"
+with patch.object(sys, "argv", ["juju-lint"]):
+    config_file = f"{Config().config_dir()}/config.yaml"
 builtin_open = open
 builtin_isfile = os.path.isfile
 
@@ -52,6 +54,7 @@ def patch_user_config(mocker):
     mocker.patch("builtins.open", my_mock_open)
 
 
[e-mail](sys, "argv", ["juju-lint"])
 def test_config_file(mocker):
     """Tests if the config entries set in the .config/juju-lint/config.yaml are correctly applied.
 
@@ -65,6 +68,7 @@ def test_config_file(mocker):
         assert config[key].get() == expected_config[key]
 
 
[e-mail](sys, "argv", ["juju-lint"])
 def test_default_config(mocker):
     """Tests if the default values are correctly read from config_default.yaml."""
     default_config = {
@@ -96,7 +100,8 @@ def test_parser_options(mocker):
         "output": {"folder": "cli/folder"},
         "format": "json",
     }
-    test_args = sys.argv + [
+    test_args = [
+        "juju-lint",
         "-F",
         cli_config["format"],
         "-l",
diff --git a/tox.ini b/tox.ini
index 8aefb78..f6d4326 100644
--- a/tox.ini
+++ b/tox.ini
@@ -48,7 +48,8 @@ commands =
 deps = {[testenv:lint]deps}
 
 [testenv:unit]
-commands = pytest {toxinidir}/tests/unit
+commands = pytest {toxinidir}/tests/unit \
+    {posargs:-v --cov --cov-report=term-missing --cov-report=html --cov-report=xml}
 deps =
     -r {toxinidir}/requirements.txt
     -r {toxinidir}/tests/unit/requirements.txt