Honour skip_module_check when filtering failures by extension module - #22
Open
dionmcm wants to merge 1 commit into
Open
Honour skip_module_check when filtering failures by extension module#22dionmcm wants to merge 1 commit into
dionmcm wants to merge 1 commit into
Conversation
Port of upstream 051e87e (VAL-531). An extension validation discards any failure whose moduleId is outside the configured module list. That is right for assertions that report failures against arbitrary components, but wrong for assertions that have already worked out which modules their failures concern - their rows were being dropped even though the assertion had deliberately selected them. Upstream's fix lets an assertion mark its own rows exempt by writing qa_result.skip_module_check, which the extractor then honours. Behaviourally inert on the corpus we currently pin (fad36466): no assertion there writes the column, so it reads back NULL, getBoolean yields false, and every row still goes through the module filter exactly as before. Verified against both corpora - the four INSERTs that set it live in three files (delta-language-unique, snapshot-language-referencedcomponent-unique-per-refset, and the inactivated-component-module stored proc), all of which arrive with the catch-up corpus 0160dd2 and none of which are among the 544 assertions production runs today. Landing the engine change first means the corpus bump does not have to carry an engine change with it. Every INSERT in both corpora names its columns explicitly, so adding a column to the entity cannot disturb them. The column itself is created by Hibernate from this entity (ddl-auto=create), so schema and reader stay in step. MysqlFailuresExtractor differs from upstream here (Collectors.toList, inlined limit), so the predicate is ported by meaning rather than as a literal diff. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018aYBXcyiozxQQGhzzhS7KG
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.
Port of upstream
051e87e(VAL-531).What upstream fixed
An extension validation discards any failure whose
moduleIdis outside the configured module list. That is right for assertions reporting failures against arbitrary components, but wrong for assertions that have already worked out which modules their failures concern — their rows were dropped even though the assertion had deliberately selected them. Upstream lets an assertion mark its own rows exempt by writingqa_result.skip_module_check, which the extractor then honours.Why this matters to us concretely
It is not only a filtering improvement — on the catch-up corpus it is currently a hard execution failure. Two of arm F's five execution errors are exactly the assertions whose
INSERTnames this column:The column does not exist, so the insert fails and the assertion cannot run. This PR creates it — Hibernate builds it from the
QAResultentity underddl-auto=create, so schema and reader stay in step.Why it is safe to land now, ahead of the corpus
Verified against both corpora:
fad364660160dd2VALUESthat a new column would breakSo on today's nightly the column reads back NULL,
getBooleanyieldsfalse, and every row goes through the module filter exactly as before. Landing the engine change first means the corpus bump does not have to carry an engine change with it.Note on fidelity to upstream
MysqlFailuresExtractorhas diverged here (Collectors.toListrather than.toList(), inlinedlimit), so the predicate is ported by meaning rather than as a literal diff. The entity and DTO changes match upstream exactly, including makingFailureDetailSerializable— which also gives the pre-existingtransientontableNamea meaning it did not have.Testing
mvn test— 88/88 green. The behavioural claim is under test in a full local run (arm H = this branch + catch-up corpus), whose prediction isexecution errors 5 → 3, clearing exactly the two assertions above.🤖 Generated with Claude Code
https://claude.ai/code/session_018aYBXcyiozxQQGhzzhS7KG