UN-3636 [FIX] Two OSS test fixes blocking the cloud rig integration tier#2202
Draft
chandrasekharan-zipstack wants to merge 2 commits into
Draft
UN-3636 [FIX] Two OSS test fixes blocking the cloud rig integration tier#2202chandrasekharan-zipstack wants to merge 2 commits into
chandrasekharan-zipstack wants to merge 2 commits into
Conversation
test_execution_serializer stubbed workflow_manager.workflow_v2.models in sys.modules at import time and never restored it. Under pytest-xdist the worker process is shared across tests, so the fake WorkflowExecution (no real Workflow) shadowed the real module for every later test on that worker, which failed to import Workflow via the URLconf. It surfaced once the rig ran real multi-worker parallelism. The stub was never needed: the suite runs under pytest-django, which loads the app registry, so the real serializer imports directly. Delete the stub (and the os.environ / sys / types scaffolding) rather than restore it in a finally, so the pattern can't be copied. The tests are unchanged and still DB-free (they drive the serializer with MagicMock execution objects). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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 |
get_sql_values_for_query fell through to f"{value}" for None, writing the
literal "None" — invalid JSON for a jsonb column (e.g. the data column on an
errored record). test_insert_into_db_with_error_postgresql was marked
xfail(strict) for exactly this; the bug is env-fragile (it reproduces on some
Postgres builds, XPASSes on others), which surfaced as a hard failure in the
cloud rig's integration tier.
Bind None as SQL NULL so the behaviour is correct and deterministic across
environments, and drop the now-obsolete strict xfail (and its unused pytest
import).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
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.
Two OSS test issues that block the cloud test-rig integration tier (which consumes OSS
main). Independent commits — splittable if preferred.1. Leaked
sys.modulesstub (test_execution_serializer)The test stubbed
workflow_manager.workflow_v2.modelsinsys.modulesat import time and never restored it. Under pytest-xdist a worker process is shared across many tests, so the fakeWorkflowExecution(no realWorkflow) shadowed the real module for every later test on that worker →cannot import name 'Workflow'via the URLconf. Latent until the rig ran real multi-worker parallelism, then deterministic (~30 failures/run).Fix: delete the stub — the suite runs under
pytest-django(app registry loaded), so the real serializer imports directly. Deleted rather than restored-in-finallyso the footgun can't be copy-pasted; it was the onlysys.modules-mutating test in the backend.2.
NoneDB value serialized as the string"None"(get_sql_values_for_query)Fell through to
f"{value}"forNone, writing the literal"None"— invalid JSON for ajsonbcolumn (e.g.dataon an errored record).test_insert_into_db_with_error_postgresqlwas markedxfail(strict)for exactly this; the bug is env-fragile (reproduces on some Postgres builds, XPASSes on others), which the cloud rig surfaced as a hardXPASS(strict)failure.Fix: bind
Noneas SQL NULL (deterministic across environments); drop the obsolete strict xfail + unusedpytestimport.Verification
-n auto(real rig config), ×2: 0Workflowimport errors, 708 passedget_sql_values_for_query({"data": None, ...})→datais SQLNULL, other values unchanged🤖 Generated with Claude Code