Test pkg_resource usage

[hmpf]

Ensure that the stuff that uses pkg_resource is tested, so that we can know it all still works after replacing pkg_resources with importlib.

Preparation for #2791.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (d5497c8) 57.04% compared to head (e9203f2) 57.04%.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #2799   +/-   ##
=======================================
  Coverage   57.04%   57.04%           
=======================================
  Files         567      567           
  Lines       41285    41289    +4     
=======================================
+ Hits        23549    23555    +6     
+ Misses      17736    17734    -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[github-actions]

Test results

     12 files       12 suites   11m 49s ⏱️
3 303 tests 3 303 ✔️ 0 💤 0 ❌
9 384 runs  9 384 ✔️ 0 💤 0 ❌

Results for commit e9203f2.

♻️ This comment has been updated with latest results.

[lunkwill42]

The tests look just fine and should be good for both coverage and refactoring, but I have my usual nitpicks about test naming 😁

[hmpf]

The final use of pkg_resources already has a test: test_installed_pyrad_can_parse_default_dictionary which is in tests/integration/statemon/radius_test.py. Why is this test in the integration tree, is pyrad.dictionary.Dictionary that expensive to run?

[hmpf]

Setup for black in github does not agree with setup for black in pre-commit, sigh.

[lunkwill42]

The final use of pkg_resources already has a test: test_installed_pyrad_can_parse_default_dictionary which is in tests/integration/statemon/radius_test.py. Why is this test in the integration tree, is pyrad.dictionary.Dictionary that expensive to run?

I can't recall specifically why it's there, but "expensiveness" has nothing to do with it. The test utilizes a 3rd party library to test the validity of a resource file that is distributed with NAV, which doesn't fit neatly into the "unit test" category in my mind.

[lunkwill42]

Setup for black in github does not agree with setup for black in pre-commit, sigh.

That must be an issue in your end. I checked out your branch, changed one letter in a test method name touched by 566dd2b and my pre-commit hook reformatted things exactly as expected by Black on GitHub.

Just a manual run of pre-commit on an unchanged version of your file also confirms it:

$ pre-commit run --files tests/unittests/config_test.py
Trim Trailing Whitespace.................................................Passed
Mixed line ending........................................................Passed
Flake8: critical.........................................................Passed
black....................................................................Failed
- hook id: black
- files were modified by this hook

reformatted tests/unittests/config_test.py

All done! ✨ 🍰 ✨
1 file reformatted.

[lunkwill42]

Provisionally accepted, as long as you get your pre-commit hooks working again :)

[hmpf]

Provisionally accepted, as long as you get your pre-commit hooks working again :)

pyproject.toml says linelength should be 88.

precommit does not set a line-length.

github actions seems to want them to be shorter than 100 since the latest lint round shortened a single line (long test-name) from 101 chars to 95 chars. So what is correct?

If a testname is longer than 88 chars, should we let black move the parameters to a new line or should we just noqa it?

[lunkwill42]

pyproject.toml says linelength should be 88.
precommit does not set a line-length.

Why would we want to set a separate line-length value just for pre-commit?
Black uses the config it finds in pyproject.toml, period. Just change
line-length in pyproject.toml and re-run pre-commit hooks and you'll see.

github actions seems to want them to be shorter than 100 since the latest lint round shortened a single line (long test-name) from 101 chars to 95 chars. So what is correct?

No, github actions doesn't want anything. It runs Black on the source file, and Black, configured with a line-length of 88 in pyproject.toml reformats the line in question according to its rules, thus (this diff is exactly the same if I run black manually, pre-commit hooks manually or look at the diff output of the GitHub superlinter action):

--- a/tests/unittests/config_test.py
+++ b/tests/unittests/config_test.py
@@ -5,7 +5,9 @@ from unittest import TestCase
 
 
 class TestConfigResourceWalk(TestCase):
-    def test_should_read_relative_paths_as_strings_from_nav_package_and_return_a_long_list_of_strings(self):
+    def test_should_read_relative_paths_as_strings_from_nav_package_and_return_a_long_list_of_strings(
+        self,
+    ):
         # result should be many, many relative paths as strings
         result = tuple(_config_resource_walk())  # generator
         self.assertTrue(len(result) > 20)

The GitHub superlinter sees that Black changed something, and raises a red flag.

Black cannot break up a long function name, so it leaves it alone - but it certainly can and will move the function arguments to the next line, according to regular Black semantics.

If a testname is longer than 88 chars, should we let black move the parameters to a new line or should we just noqa it?

Leave Black alone to do its thing, that's what it's there for :)

[sonarcloud]

Quality Gate passed

Issues
0 New issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud