fix: enable strict positional enforcement for the test suite under pytest - #2802
Open
skippdot wants to merge 2 commits into
Open
fix: enable strict positional enforcement for the test suite under pytest#2802skippdot wants to merge 2 commits into
skippdot wants to merge 2 commits into
Conversation
…test - Set `positional_parameters_enforcement = POSITIONAL_EXCEPTION` at import time in tests/__init__.py and delete the nose-era `setup_package()` hook, which pytest never calls, leaving the suite in warning-only mode - Save and restore the enforcement setting in PositionalTests, whose methods mutate the module-level flag and previously leaked whatever value the last-run test set, making other tests pass or fail depending on order - The suite only passed in full runs by accident: alphabetical ordering left `test_usage` (EXCEPTION) as the last write; any other order, or running test_discovery.py alone, failed `test_tests_should_be_run_with_strict_positional_enforcement` Fixes googleapis#2755
There was a problem hiding this comment.
Code Review
This pull request updates the test suite initialization to set positional parameter enforcement at import time, ensuring compatibility with pytest, and adds state restoration to PositionalTests to prevent side effects. The reviewer suggests using self.addCleanup in setUp as a more robust and idiomatic way to restore the module-level enforcement setting instead of using a separate tearDown method.
- Replace the setUp/tearDown pair in PositionalTests with a single setUp registering addCleanup, as suggested in review - Cleanup now runs even if setUp or the test itself fails midway
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2755
The test suite is written against strict positional parameter enforcement (
TypeErroron violation), but that mode was only ever switched on inside the nose-erasetup_package()hook intests/__init__.py— a hook pytest never calls. Under pytest the suite silently runs in the default warning-only mode.The reason full runs still pass is an accident of ordering, which is the second half of the problem: the
PositionalTestsmethods intests/test__helpers.pymutate the module-levelpositional_parameters_enforcementflag and never restore it. Alphabetical execution leavestest_usage(which setsEXCEPTION) as the last write, sotests/test_discovery.pyhappens to run strict. Any other order breaks it — reproducible on an unmodifiedmain:as is running
tests/test_discovery.pyon its own.Changes
tests/__init__.py: setpositional_parameters_enforcement = util.POSITIONAL_EXCEPTIONat import time (using the constant rather than the raw string) and delete the deadsetup_package()hook — nothing references it.tests/test__helpers.py: addsetUp/tearDowntoPositionalTeststo save and restore the enforcement flag, so these tests stop leaking global state into the rest of the suite.Two files, +13/−4.
Verification (Python 3.14)
test_enforcement_ignoreordered before the strict testtests/test_discovery.pyalonetests/foldertest_discovery.pybeforetest__helpers.py)black --checkclean;flake8 --select=E9,F63,F7,F82(the set CI enforces) clean.