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 broken navclean and navsynctypes scripts #2875

Merged
merged 4 commits into from
Mar 15, 2024

Conversation

lunkwill42
Copy link
Member

@lunkwill42 lunkwill42 commented Mar 15, 2024

The navclean and navsynctypes scripts were broken in NAV 5.9.0, after the method of script installation was modernized.

This PR changes the test suite to test-run the scripts defined for installation in pyproject.toml, rather than running them in-place in the nav.bin package, to ensure that they are truly compatible with being executed by end-users in an installed system.

Fixes #2874

This replaces the `test_binary_runs` test with `test_script_runs` test.
The former would just enumerate all the modules present in the `nav.bin`
packages, extract their test args and run them directly from that
location.  However, not all the scripts here are compatible with being
invoked from a pip-installed bin-directory shim that calls their main
method directly.

I.e. this updates the test suite to attempt to call all the defined
NAV scripts in the manner they would be run in an installed system,
rather than in an artificial manner.  This should detect additional
problems with the scripts.
This required-but-unused argument to main prevented `navclean` from
running properly when invoked from the binary shim installed by
pip/setuptools.

Fixes Uninett#2874
This required-but-unused argument to main prevented `navsynctypes` from
running properly when invoked from the binary shim installed by
pip/setuptools.  The argparser is only there to provide a nice help
screen if the `--help` argument is provided.
@lunkwill42 lunkwill42 added the bug label Mar 15, 2024
@lunkwill42 lunkwill42 requested a review from hmpf March 15, 2024 10:34
@lunkwill42 lunkwill42 self-assigned this Mar 15, 2024
@lunkwill42 lunkwill42 changed the base branch from master to 5.9.x March 15, 2024 10:34
Copy link

github-actions bot commented Mar 15, 2024

Test results

     12 files       12 suites   11m 26s ⏱️
3 320 tests 3 320 ✔️ 0 💤 0
9 435 runs  9 435 ✔️ 0 💤 0

Results for commit 005d1aa.

♻️ This comment has been updated with latest results.

Copy link

codecov bot commented Mar 15, 2024

Codecov Report

Attention: Patch coverage is 60.00000% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 56.64%. Comparing base (efb9fac) to head (005d1aa).

Files Patch % Lines
python/nav/bin/navclean.py 50.00% 1 Missing ⚠️
python/nav/bin/navsynctypes.py 66.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##            5.9.x    #2875      +/-   ##
==========================================
- Coverage   56.70%   56.64%   -0.07%     
==========================================
  Files         603      603              
  Lines       43983    43984       +1     
==========================================
- Hits        24942    24916      -26     
- Misses      19041    19068      +27     

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

@hmpf
Copy link
Contributor

hmpf commented Mar 15, 2024

Python gets toml in the stdlib in 3.11: https://docs.python.org/3/library/tomllib.html

Since we still can't run on 3.11 we don't need to hedge.

Aside, we should copy direct dependencies from requirements.txt to pyproject.toml [project.dependencies], some other time.

@lunkwill42 lunkwill42 merged commit 142309b into Uninett:5.9.x Mar 15, 2024
9 of 10 checks passed
@lunkwill42 lunkwill42 deleted the bugfix/test-scripts branch March 15, 2024 10:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Navclean cron error
2 participants