-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Description
Failure Details
- Run: 22835872084
- Commit:
55a0a5870e681fa7c9f11bce86425ed7864c6d38 - PR: Validate constant directive values on variable definitions (#3927) #4271 – "Validate constant directive values on variable definitions (validate constant directive values #3927)"
- Branch:
claude/fix-issue-3927-mrgkP
Failed Jobs and Errors
| Job | Conclusion |
|---|---|
| Per-Class Coverage Gate | ❌ failure |
| allBuildAndTestSuccessful | ❌ failure (depends on above) |
| All other jobs (build, test, javadoc) | ✅ success |
The Per-Class Coverage Gate failed at step Enforce Per-Class Coverage Gate. This gate compares per-class branch/line/method coverage from the JaCoCo report against the baseline in test-baseline.json. It fails if any class regresses by more than 0.05%.
Root Cause
PR #4271 adds validateVariableNotAllowedInConstantDirective() to OperationValidator.java. A specific code path in this new method is left uncovered by tests, causing branch coverage for graphql.validation.OperationValidator to drop below the baseline.
The uncovered path is:
private void validateVariableNotAllowedInConstantDirective(
VariableReference variableReference, List(Node) ancestors) {
boolean inDirective = false;
for (int i = ancestors.size() - 1; i >= 0; i--) {
Node ancestor = ancestors.get(i);
if (ancestor instanceof Directive) {
inDirective = true; // ← covered
} else if (ancestor instanceof VariableDefinition) {
if (inDirective) { ... error } // ← covered
return;
} else if (ancestor instanceof OperationDefinition || ancestor instanceof Document) {
return; // ← OperationDefinition path covered only when inDirective=false
}
}
}The missing branch: inDirective = true (a Directive was seen) → then the loop hits OperationDefinition (not VariableDefinition) → return without error.
This path represents a variable reference inside a directive on a field (not a variable definition). The test intended to cover this is:
// src/test/groovy/graphql/validation/VariablesNotAllowedInConstantDirectivesTest.groovy, line 66
def "variable reference in field directive argument is accepted"() {
given:
def query = '''
query ($v: Int) { x `@strDir`(arg: "hello") } // ← BUG: uses "hello" constant, NOT $v
'''The test uses "hello" (a constant), so there is no VariableReference node in the AST. validateVariableNotAllowedInConstantDirective is never called. The branch where a variable IS in a field directive (and should be allowed) is never exercised.
The baseline coverage for graphql.validation.OperationValidator is:
- Branch:
{covered: 601, missed: 51}= 92.17%
The new code adds new conditional branches; with the above test gap, the branch coverage drops below 92.17%, triggering the gate.
Recommended Fix
In src/test/groovy/graphql/validation/VariablesNotAllowedInConstantDirectivesTest.groovy, fix the misleadingly-named test to actually use a variable reference in the field directive:
// Line ~66: change the query from using "hello" to $v
def "variable reference in field directive argument is accepted"() {
given:
def query = '''
query ($v: String) { x `@strDir`(arg: $v) } // ← use $v here, not "hello"
'''
when:
traverse(query)
then:
errorCollector.errors.isEmpty()
}This ensures validateVariableNotAllowedInConstantDirective is called with a VariableReference whose ancestors include a Directive followed (up-chain) by an OperationDefinition (not a VariableDefinition), covering the missing branch.
- Fix
VariablesNotAllowedInConstantDirectivesTest.groovyline ~68: change query fromquery ($v: Int) { x@strDir(arg: "hello") }toquery ($v: String) { x@strDir(arg: $v) } - Re-run CI to confirm the coverage gate passes
- If branch coverage still doesn't meet the baseline after the test fix, also update
test-baseline.json
Generated by CI Failure Doctor
To install this workflow, run
gh aw add githubnext/agentics/workflows/ci-doctor.md@ee50a3b7d1d3eb4a8c409ac9409fd61c9a66b0f5. View source at https://github.com/githubnext/agentics/tree/ee50a3b7d1d3eb4a8c409ac9409fd61c9a66b0f5/workflows/ci-doctor.md.