[MISC] Fail rig groups that inherit an unrelated pytest config; skip order-dependent connector tests#2205
Draft
chandrasekharan-zipstack wants to merge 3 commits into
Draft
[MISC] Fail rig groups that inherit an unrelated pytest config; skip order-dependent connector tests#2205chandrasekharan-zipstack wants to merge 3 commits into
chandrasekharan-zipstack wants to merge 3 commits into
Conversation
…nfig `cd workdir && pytest` does not pin config to workdir — pytest walks up until it finds one. A group whose workdir is a nested project without its own config therefore silently adopts its parent's addopts (coverage targets, --strict-config) and fails for reasons unrelated to its tests. The failure is opaque: a usage error, or zero tests collected. Resolve the config file the same way pytest does and fail the group up front with a message naming the offending file and the fix. Inheriting the repo-root config stays allowed — that is the normal monorepo case, and two groups rely on it today. Verified against all 18 existing pytest groups: none are rejected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnLaN45ShBbcCXWZkqCThz
These tests share a fixed table (or remote path) across test methods with no working teardown — several tearDown bodies are a bare `pass` with the drop statements commented out. Results therefore depend on execution order, and the SharePoint read test consumes what the write test leaves behind. They are inert today only because the credentials they need are not provisioned, so the breakage would surface as a surprise the moment someone adds them. Skip explicitly, with the reason stated, so enabling credentials is a deliberate step rather than an accident. The postgres variant is deliberately untouched — it runs today and is being fixed rather than skipped. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnLaN45ShBbcCXWZkqCThz
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
chandrasekharan-zipstack
changed the title
…gres The lane reported 141 tests, 141 skipped: the rig provisions a testcontainers Postgres and exports DB_*, but the workers fixtures read TEST_DB_* and so fell back to the dev-compose defaults, and nothing ever applied the pg_queue schema. Mirror the provisioned DB_* onto TEST_DB_* for groups requiring postgres (defaulting DB_SCHEMA to public — a bare container has no other schema), and apply defaults.postgres_migrate once after the container is up and before any group runs. Schema is a property of the database rather than of a group, and guarding on the provisioned URL keeps it a no-op under the compose runtime, where the platform's own backend migrates on start. Django migrate rather than DDL in a fixture: the lane asserts against the real generated CheckConstraint, which hand-written DDL would render self-referential. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RnLaN45ShBbcCXWZkqCThz
|
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
1 participant
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.
Why
Two related test-rig defects surfaced while wiring cloud CI onto the rig.
1. A group can silently inherit another project's pytest config.
The rig runs
cd <workdir> && pytest <paths>and never pins the config file. pytest resolves config by walking up from the workdir, so a group whose workdir is a nested project without its own config adopts its parent'saddopts— coverage targets,--strict-config— and fails for reasons that have nothing to do with its tests. Observed symptom:ERROR: Unknown config option: asyncio_modefollowed byno tests ran, and a tier exit 4.--rootdirdoes not fix this (verified empirically — it moves rootdir but leaves ini-file discovery alone). Only an own config section, or-c, does.This matters beyond the one case:
workers/plugins/holds 11 nested projects, each a future instance of the same trap.2. Several connector tests are order-dependent and dormant.
They share a fixed table (or remote path) across test methods with no working teardown — several
tearDownbodies are a barepasswith the drops commented out. They are inert only because their credentials aren't provisioned, so the breakage would appear as a surprise the moment someone adds them.What
_resolve_pytest_configfile()mirrors pytest's upward search;_config_isolation_error()fails a group up front when the resolved config belongs to neither its workdir nor the repo root, with a message naming the offending file and the fix.unit-core/unit-platform-servicerely on it today.@unittest.skipwith the reason stated on the 7 dormant connector test classes and the 2 SharePoint tests, so enabling credentials becomes a deliberate step.Verification
🤖 Generated with Claude Code
https://claude.ai/code/session_01RnLaN45ShBbcCXWZkqCThz