Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix various test skip logic issues #3015

Merged
merged 9 commits into from
Sep 27, 2024
Merged

Fix various test skip logic issues #3015

merged 9 commits into from
Sep 27, 2024

Conversation

lunkwill42
Copy link
Member

This PR contains various fixes to the test suite, all extracted from #2675:

  • Fixes issues with broken test skip logic
  • Lets more tests be skipped if certain requirements aren't present. The test suite was originally written to run mostly in the customized test environment defined in a Docker image, and would just pre-suppose a bunch of non-NAV command line tools to always be present. Some or all of these requirements could be missing if tests are run in a dev's local environment. Instead of failing these tests because of missing requirements, they should be marked as skipped.

`sudo -nv` can be used to verify whether the sudo command is prepared
to execute commands non-interactively, i.e. it will not ask for a
password.

If the password has already been cached by previous sudo commands, or
sudo is configured to give passwordless access, we can move on.
Otherwise, interactive sudo or lack of sudo access cannot be used to
run tests in an automated fashion.
Some pping tests require starting up the pping daemon and killing it
after some time. These tests seem to rely on the external `timeout`
program, which isn't always available on a system (e.g. it's not
available on MacOS).
The python tidylib module loads the system library dynamically.  This
ensure we skip the HTML validation tests if tidylib isn't properly
installed.
If nbtscan is not available (it is, after all, an optional dependency
of NAV), netbiostracker.py cannot run, so we might as well skip it.
These tests are supposed to be skipped if we CANNOT be root, not if we
*can*.
Copy link

github-actions bot commented Sep 24, 2024

🦙 MegaLinter status: ✅ SUCCESS

Descriptor Linter Files Fixed Errors Elapsed time
✅ PYTHON black 992 0 11.13s
✅ PYTHON ruff 987 0 0.09s

See detailed report in MegaLinter reports

MegaLinter is graciously provided by OX Security

Copy link

github-actions bot commented Sep 24, 2024

Test results

    9 files      9 suites   8m 21s ⏱️
2 128 tests 2 128 ✅ 0 💤 0 ❌
3 995 runs  3 995 ✅ 0 💤 0 ❌

Results for commit eabd099.

♻️ This comment has been updated with latest results.

Copy link

codecov bot commented Sep 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 60.45%. Comparing base (bed07fe) to head (eabd099).
Report is 417 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3015   +/-   ##
=======================================
  Coverage   60.45%   60.45%           
=======================================
  Files         605      605           
  Lines       43801    43801           
  Branches       48       48           
=======================================
  Hits        26482    26482           
  Misses      17307    17307           
  Partials       12       12           

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

@lunkwill42 lunkwill42 requested a review from a team September 24, 2024 11:46
@lunkwill42 lunkwill42 marked this pull request as ready for review September 24, 2024 11:46
@lunkwill42 lunkwill42 self-assigned this Sep 24, 2024
@lunkwill42 lunkwill42 marked this pull request as draft September 24, 2024 12:16
@lunkwill42
Copy link
Member Author

Returning to draft mode- This did not have the intended effect, as many tests were now skipped for reasons unknown...

Too broad exception catching caused a `NameError` to be masked
and all the tests to be skipped: The `get_root_method` function was
not available at test definition time, since it was defined below the
tests.

This narrows the exception catcher and moves the `get_root_method()`
before the first test is defined.
LDAP *is* marked as an optional dependency, so the test suite should
still complete without it.
@lunkwill42 lunkwill42 marked this pull request as ready for review September 24, 2024 13:39
@lunkwill42
Copy link
Member Author

Found several more issues that were revealed only by pushing this PR to GitHub in the first place :-)

Copy link

@lunkwill42 lunkwill42 merged commit 2d74945 into master Sep 27, 2024
14 checks passed
@lunkwill42 lunkwill42 deleted the test/fix-skip-issue branch September 27, 2024 11:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants