Skip to content

Clean up various pytest warnings#22472

Open
mvdbeek wants to merge 5 commits intogalaxyproject:devfrom
mvdbeek:warnings/pytest-hygiene
Open

Clean up various pytest warnings#22472
mvdbeek wants to merge 5 commits intogalaxyproject:devfrom
mvdbeek:warnings/pytest-hygiene

Conversation

@mvdbeek
Copy link
Copy Markdown
Member

@mvdbeek mvdbeek commented Apr 13, 2026

See commits for details

How to test the changes?

(Select all options that apply)

  • I've included appropriate automated tests.
  • This is a refactoring of components with existing test coverage.
  • Instructions for manual testing are as follows:
    1. [add testing steps and prerequisites here if you didn't write automated tests covering all your changes]

License

  • I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.

@mvdbeek mvdbeek added the kind/refactoring cleanup or refactoring of existing code, no functional changes label Apr 13, 2026
@github-actions github-actions bot added this to the 26.1 milestone Apr 13, 2026
mvdbeek added 3 commits April 14, 2026 15:51
Pytest 8 warns (PytestReturnNotNoneWarning) when a test function
returns a non-None value, and will turn it into an error in a
future major version. Two API tests did this:

 - test_create_history_json returned the create_response dict;
   no callers, so just drop the return.
 - test_create_valid was both a test AND a factory that other
   tests called to obtain a freshly-created group. Extract the
   factory into _create_valid_group() and have the actual test
   just call it without returning — callers now use the helper.
lib/galaxy_test/base/decorators.py dynamically applies
pytest.mark.requires_<tag> (for tag in admin, celery, new_history,
new_library, new_published_objects, new_user), but none of those
names were registered in pytest.ini — pytest emitted
PytestUnknownMarkWarning for each.

A stale 'require_new_history' (singular) entry was also present;
drop it in favour of the plural form that matches what the code
actually uses.
Pytest's default collection heuristic picks up any class whose name
starts with 'Test'; when it tries to collect a class that has a
__init__ / __new__ (e.g. a TypedDict or namedtuple) it emits a
PytestCollectionWarning and skips it. Two such classes were being
warned on:

 - galaxy.tool_util_models.TestJobDict (TypedDict) → JobTestDict
 - test.integration.test_datatype_upload.TestData (namedtuple)
   → DatatypeUploadCase

Update all callers accordingly.
@mvdbeek mvdbeek force-pushed the warnings/pytest-hygiene branch from 8acbc84 to 3b730e3 Compare April 14, 2026 13:51
mvdbeek added 2 commits April 14, 2026 19:12
Tests were annotated with `@pytest.mark.require_new_history` (singular),
which is not registered in pytest.ini (hence PytestUnknownMarkWarning) and,
more importantly, does not attach the `__required_galaxy_features`
attribute that populators.test_history_for reads via has_requirement().
As a result, those tests were silently reusing histories instead of
getting a freshly-created one.

Replace all such marker usages with the `requires_new_history` decorator
from galaxy_test.base.decorators, which both registers the canonical
`requires_new_history` marker and attaches the requirement attribute.
Drop `--cov-report term` so pytest no longer prints the per-file coverage
table, and instead emit a single `Total coverage: N%` line after the run
via `coverage report --format=total`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/testing/api area/testing/integration area/testing kind/refactoring cleanup or refactoring of existing code, no functional changes

Projects

Status: Needs Review

Development

Successfully merging this pull request may close these issues.

2 participants