Skip to content

Conversation

@willmurphyscode
Copy link
Contributor

Updates the tox environment list to include 3.14 for testing in CI.
Updates any dependencies that would not work with 3.14 until they will.

A number of additional changes in here to make CI better:

  • fix some unit tests that were using real sleeps
  • cache the JSON schema validator (per schema url) in tests (the ubuntu unit tests were making the same schema validator ~180 times)
  • fix a bunch of linter errors
  • separate the html coverage report generating into its own Taskfile target
  • pass --durations=10 to pytest (prints the 10 slowest tests and their runtimes)
  • did some work to suppress warnings about leaked sqlite connections

This last probably merits some explanation. Starting in Python 3.13 or so, if a sqlite connection object is garbage collected before .close() was called, it emits a warning. However, in SQLAlchemy, the underlying sqlite connection objects are pooled, and close() just frees them to be closed asynchronously. This means that in unit tests, the garbage collector and the stale connection pool cleanup are racing, and if the garbage collector wins (loses?) we get a warning.

I tried a number of lighter approaches to suppress these warnings, and settled on the current position as least bad, but I'm definitely open to feedback. Also, vunnel is a cron job, not a server, which makes me more tolerant of the risk that our unit tests miss an actual resource leak. I would love for someone to show me a better set of tradeoffs here.

Updates the tox environment list to include 3.14 for testing in CI.
Updates any dependencies that would not work with 3.14 until they will.

Signed-off-by: Will Murphy <[email protected]>
Start passing --durations=10 to pytest (so that the 10 slowest tests are
printed after each run). Then fix the worst offenders, which happened to
be tests that were actually hitting unmocked sleeps.

Signed-off-by: Will Murphy <[email protected]>
One test was instantiating the same schema validator ~180 times,
resulting in a 10s unit test.

Signed-off-by: Will Murphy <[email protected]>
Add a session-scoped pytest fixture that patches SQLAlchemy's
create_engine to use NullPool for all SQLite connections during test
runs. This ensures connections are closed immediately rather than being
held in a pool, making connection leaks deterministic and visible.

The warnings were caused by SQLAlchemy's default connection pooling
holding sqlite3.Connection objects open until garbage collection, which
triggered ResourceWarnings when pytest's unraisableexception plugin
ran gc.collect() at the end of test sessions.

Production code remains unchanged - the default connection pooling is
preserved for the read/write-heavy workloads in vunnel's fix date
databases. Only tests get NullPool behavior via the conftest.py fixture.

Also:
- Remove --cov-report html from default unit test target (CI doesn't
  use it, and it adds overhead)
- Add unit-coverage target for when HTML reports are needed

Signed-off-by: Will Murphy <[email protected]>
- Remove deprecated UP038 rule from ruff ignore list (rule no longer exists)
- Convert Optional[X] to X | None syntax (Python 3.10+, safe since we require 3.13+)
- Remove now-unnecessary noqa comments (UP007, S320, S603)
- Add noqa comment explaining intentional lazy import in cli.py
- Move imports to top-level where they were unnecessarily deferred
  (secureos/parser.py, grype_db_first_observed.py)
- Prefix unused unpacked variables with underscore
  (amazon, oracle, ubuntu, rpm parsers)
- Simplify workspace.py to use standard library importlib.metadata
  directly (no Python < 3.8 fallback needed)
- Remove unused imports (Optional, types)

Signed-off-by: Will Murphy <[email protected]>
@willmurphyscode willmurphyscode self-assigned this Feb 3, 2026
Previously, it was possible to enable a python version in tox.ini and
then believe it was supported without the tests running in CI.

Signed-off-by: Will Murphy <[email protected]>
@willmurphyscode willmurphyscode marked this pull request as ready for review February 3, 2026 22:33
Copy link
Contributor

@spiffcs spiffcs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Green 🟢 - good to add the label and run the quality gate tests here. I have no suggested changed.

request_timeout: int = 125
request_retry_count: int = 10
api_key: Optional[str] = "env:NVD_API_KEY" # noqa: UP007
api_key: str | None = "env:NVD_API_KEY"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice clean up here

import_results_path: Optional[str] = None # noqa: UP007 - breaks mashumaro
import_results_enabled: Optional[bool] = None # noqa: UP007 - breaks mashumaro
user_agent: Optional[str] = None # noqa: UP007 - breaks mashumaro
import_results_host: str | None = None
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow - I said in a previous comment, but this is way more readable thanks for the cleanup work on these

@willmurphyscode willmurphyscode added the run-pr-quality-gate Triggers running of quality gate on PRs label Feb 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

run-pr-quality-gate Triggers running of quality gate on PRs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants