From 928af4fb06c524dcc2c1a33a12d832df0d2cead2 Mon Sep 17 00:00:00 2001 From: Alex Skrypnyk Date: Sun, 5 Jul 2026 14:00:07 +1000 Subject: [PATCH 1/4] [#664] Fixed 'accessibilityRenderJunit()' emitting advisory violations as JUnit failures. The renderer now gates '' output by the scenario's effective threshold, mirroring the pass/fail gate. Advisory violations (threshold 'never', e.g. '@accessibility:warning') are recorded as passing testcases carrying the finding in '', so a passing advisory run writes a report with zero failures instead of one that reddens a JUnit-consuming CI check. The 'tests' and 'failures' counts now reflect the actual emitted '' elements, one per affected node. Claude-Session: https://claude.ai/code/session_01JTm9W2rNMjgzxqPVD6BGFi --- src/AccessibilityTrait.php | 43 +++++---- tests/behat/bootstrap/BehatCliContext.php | 38 ++++++++ tests/behat/features/accessibility.feature | 23 +++++ tests/phpunit/src/AccessibilityTraitTest.php | 93 ++++++++++++++++++++ 4 files changed, 182 insertions(+), 15 deletions(-) diff --git a/src/AccessibilityTrait.php b/src/AccessibilityTrait.php index aad639de..6c89d76d 100644 --- a/src/AccessibilityTrait.php +++ b/src/AccessibilityTrait.php @@ -895,8 +895,18 @@ protected function accessibilityRenderIssueList(string $heading, string $css_cla /** * Render the scenario-level JUnit XML report from collected results. + * + * Violations are gated by the scenario's effective threshold, exactly as the + * pass/fail gate is: only violations meeting the threshold are serialised as + * `` cases, so an advisory run (threshold `never`) writes a report + * with zero failures instead of one that reddens a JUnit-consuming CI check. + * Violations below the threshold are recorded as passing cases carrying the + * finding in ``, so they stay visible without failing the report. + * The `tests` and `failures` counts reflect the actual emitted `` + * elements, one per affected node. */ protected function accessibilityRenderJunit(): string { + $threshold = $this->accessibilityEffectiveThreshold(); $suites_xml = ''; $total_tests = 0; $total_failures = 0; @@ -906,13 +916,12 @@ protected function accessibilityRenderJunit(): string { $violations = $r['result']['violations'] ?? []; $passes = $r['result']['passes'] ?? []; - $tests = count($violations) + count($passes); - $failures = count($violations); - $total_tests += $tests; - $total_failures += $failures; - $cases_xml = ''; + $tests = 0; + $failures = 0; + foreach ($violations as $v) { + $failing = $this->accessibilityFilterViolations([$v], $threshold) !== []; $rule_id = (string) ($v['id'] ?? 'unknown'); $impact = (string) ($v['impact'] ?? 'unknown'); $help = (string) ($v['help'] ?? ''); @@ -921,26 +930,30 @@ protected function accessibilityRenderJunit(): string { foreach ($v['nodes'] ?? [] as $node) { $target = static::accessibilityStringifyTarget($node['target'] ?? []); $html = trim((string) ($node['html'] ?? '')); - - $message = sprintf('[%s] %s', $impact, $help); $details = sprintf("URL: %s\nRule: %s\nTarget: %s\nHTML: %s\nDocs: %s", $url, $rule_id, $target, $html, $help_url); + $classname = htmlspecialchars('accessibility.' . $rule_id, ENT_XML1 | ENT_QUOTES); + $name = htmlspecialchars($target ?: $rule_id, ENT_XML1 | ENT_QUOTES); + $tests++; + + if (!$failing) { + $cases_xml .= sprintf('%s', $classname, $name, htmlspecialchars('[advisory] ' . $details, ENT_XML1 | ENT_QUOTES)); + continue; + } - $cases_xml .= sprintf( - '%s', - htmlspecialchars($rule_id, ENT_XML1 | ENT_QUOTES), - htmlspecialchars($target ?: $rule_id, ENT_XML1 | ENT_QUOTES), - htmlspecialchars($impact, ENT_XML1 | ENT_QUOTES), - htmlspecialchars($message, ENT_XML1 | ENT_QUOTES), - htmlspecialchars($details, ENT_XML1 | ENT_QUOTES) - ); + $failures++; + $cases_xml .= sprintf('%s', $classname, $name, htmlspecialchars($impact, ENT_XML1 | ENT_QUOTES), htmlspecialchars(sprintf('[%s] %s', $impact, $help), ENT_XML1 | ENT_QUOTES), htmlspecialchars($details, ENT_XML1 | ENT_QUOTES)); } } foreach ($passes as $p) { $rule_id = (string) ($p['id'] ?? 'unknown'); + $tests++; $cases_xml .= sprintf('', htmlspecialchars($rule_id, ENT_XML1 | ENT_QUOTES), htmlspecialchars($rule_id, ENT_XML1 | ENT_QUOTES)); } + $total_tests += $tests; + $total_failures += $failures; + $suites_xml .= sprintf('%s', htmlspecialchars((string) $url, ENT_XML1 | ENT_QUOTES), $tests, $failures, $cases_xml); } diff --git a/tests/behat/bootstrap/BehatCliContext.php b/tests/behat/bootstrap/BehatCliContext.php index 6a7ae603..52bf281a 100644 --- a/tests/behat/bootstrap/BehatCliContext.php +++ b/tests/behat/bootstrap/BehatCliContext.php @@ -215,6 +215,44 @@ public function fileMatchingShouldExist($pattern) Assert::assertNotEmpty($matches, sprintf('No file matching "%s" was found in the working directory.', $pattern)); } + /** + * Checks that the first file matching a glob pattern contains a substring. + */ + #[Then('a file matching :pattern should contain:')] + public function fileMatchingShouldContain(string $pattern, PyStringNode $text): void + { + $matches = glob($this->workingDir . DIRECTORY_SEPARATOR . $pattern) ?: []; + Assert::assertNotEmpty($matches, sprintf('No file matching "%s" was found in the working directory.', $pattern)); + + $content = (string) file_get_contents($matches[0]); + $needle = trim((string) $text); + + if (str_contains($content, $needle)) { + return; + } + + throw new UnexpectedValueException(sprintf('File "%s" does not contain "%s".', $matches[0], $needle)); + } + + /** + * Checks that the first file matching a glob pattern lacks a substring. + */ + #[Then('a file matching :pattern should not contain:')] + public function fileMatchingShouldNotContain(string $pattern, PyStringNode $text): void + { + $matches = glob($this->workingDir . DIRECTORY_SEPARATOR . $pattern) ?: []; + Assert::assertNotEmpty($matches, sprintf('No file matching "%s" was found in the working directory.', $pattern)); + + $content = (string) file_get_contents($matches[0]); + $needle = trim((string) $text); + + if (!str_contains($content, $needle)) { + return; + } + + throw new UnexpectedValueException(sprintf('File "%s" unexpectedly contains "%s".', $matches[0], $needle)); + } + /** * Sets specified ENV variable. * diff --git a/tests/behat/features/accessibility.feature b/tests/behat/features/accessibility.feature index 7c554f28..f86eb693 100644 --- a/tests/behat/features/accessibility.feature +++ b/tests/behat/features/accessibility.feature @@ -100,3 +100,26 @@ Feature: Check that AccessibilityTrait works When I run "behat --no-colors" Then it should pass And a file matching ".logs/test_results/accessibility/accessibility_report_*.html" should exist + + @trait:AccessibilityTrait + Scenario: Warning mode writes a JUnit report with no failures + Given some behat configuration + And scenario steps tagged with "@javascript @accessibility:warning": + """ + Given I visit "/sites/default/files/accessibility_violations.html" + Then I should see "Inaccessible Page" + """ + When I run "behat --no-colors" + Then it should pass + And a file matching ".logs/test_results/accessibility/junit-*.xml" should contain: + """ + failures="0" + """ + And a file matching ".logs/test_results/accessibility/junit-*.xml" should not contain: + """ + + """ diff --git a/tests/phpunit/src/AccessibilityTraitTest.php b/tests/phpunit/src/AccessibilityTraitTest.php index 3c0b12b1..e86bdefa 100644 --- a/tests/phpunit/src/AccessibilityTraitTest.php +++ b/tests/phpunit/src/AccessibilityTraitTest.php @@ -346,6 +346,65 @@ public function testAggregateCaptureFormatsUrlsAndRecordsEntry(): void { $this->assertSame('/captured/dir', AccessibilityTraitTestImplementation::testGetAggregateReportDir()); } + #[DataProvider('dataProviderRenderJunitFailuresByThreshold')] + public function testRenderJunitFailuresByThreshold(?string $threshold, int $expected_failures): void { + $xml = $this->testObject->testRenderJunit(static::createJunitResults(), 'Feature', 'Scenario', $threshold); + + // Only violations meeting the effective threshold become cases. + $this->assertSame($expected_failures, substr_count($xml, ' elements reflects the threshold.'); + // Every emitted testcase is counted, so tests stays 5 (3 violations + 2 + // passes) regardless of how many are failures versus advisory. + $this->assertStringContainsString(sprintf('', $expected_failures), $xml); + // The suite-level and file-level failure counts agree. + $this->assertSame(2, substr_count($xml, sprintf('failures="%d"', $expected_failures))); + } + + public static function dataProviderRenderJunitFailuresByThreshold(): array { + return [ + 'warning (never) fails on nothing' => ['never', 0], + 'default (null) resolves to any and fails on all' => [NULL, 3], + 'any fails on all' => ['any', 3], + 'critical fails on the critical only' => ['critical', 1], + 'serious fails on critical and serious' => ['serious', 2], + 'moderate fails on all three' => ['moderate', 3], + ]; + } + + public function testRenderJunitWarningKeepsAdvisoryViolationsVisibleWithoutFailing(): void { + $xml = $this->testObject->testRenderJunit(static::createJunitResults(), 'Feature', 'Scenario', 'never'); + + // Advisory mode: no violation is serialised as a failure. + $this->assertStringNotContainsString('assertStringContainsString('failures="0"', $xml); + // The findings stay visible as passing testcases carrying . + $this->assertStringContainsString('', $xml); + $this->assertStringContainsString('classname="accessibility.image-alt"', $xml); + $this->assertStringContainsString('classname="accessibility.link-name"', $xml); + } + + public function testRenderJunitCountsEveryEmittedTestcase(): void { + $results = [ + [ + 'url' => '/', + 'rules' => 'wcag2a', + 'result' => [ + 'violations' => [ + ['id' => 'image-alt', 'impact' => 'critical', 'help' => 'Images must have alternative text', 'helpUrl' => 'https://example.com/image-alt', 'nodes' => [['target' => ['img.a'], 'html' => ''], ['target' => ['img.b'], 'html' => '']]], + ], + 'incomplete' => [], + 'passes' => [['id' => 'document-title'], ['id' => 'html-has-lang'], ['id' => 'region']], + ], + ], + ]; + + $xml = $this->testObject->testRenderJunit($results, 'Feature', 'Scenario', 'any'); + + // One per affected node (2), plus three passing testcases: the + // tests and failures attributes count actual emitted elements. + $this->assertSame(2, substr_count($xml, '}> + * A single-page result: critical, serious and moderate violations (one + * affected node each) alongside two passing rules. + */ + protected static function createJunitResults(): array { + return [ + [ + 'url' => '/', + 'rules' => 'wcag2a,wcag2aa', + 'result' => [ + 'violations' => [ + ['id' => 'image-alt', 'impact' => 'critical', 'help' => 'Images must have alternative text', 'helpUrl' => 'https://example.com/image-alt', 'nodes' => [['target' => ['img'], 'html' => '']]], + ['id' => 'link-name', 'impact' => 'serious', 'help' => 'Links must have discernible text', 'helpUrl' => 'https://example.com/link-name', 'nodes' => [['target' => ['a'], 'html' => '']]], + ['id' => 'region', 'impact' => 'moderate', 'help' => 'All page content should be contained by landmarks', 'helpUrl' => 'https://example.com/region', 'nodes' => [['target' => ['div'], 'html' => '
']]], + ], + 'incomplete' => [], + 'passes' => [['id' => 'document-title'], ['id' => 'html-has-lang']], + ], + ], + ]; + } + } /** @@ -560,4 +644,13 @@ public function testCapture(array $results, string $feature, string $scenario, s $this->accessibilityAggregateCapture($dir); } + public function testRenderJunit(array $results, string $feature, string $scenario, ?string $threshold = NULL): string { + $this->accessibilityResults = $results; + $this->accessibilityFeatureName = $feature; + $this->accessibilityScenarioName = $scenario; + $this->accessibilityScenarioThreshold = $threshold; + + return $this->accessibilityRenderJunit(); + } + } From fce5763f50f74edce1965c7a1eae4e650259470e Mon Sep 17 00:00:00 2001 From: Alex Skrypnyk Date: Sun, 5 Jul 2026 14:07:49 +1000 Subject: [PATCH 2/4] Extracted a shared file-content assertion helper and made glob selection deterministic. The two 'a file matching ... should (not) contain:' steps now delegate to one helper that sorts the glob matches before reading, so the inspected file is stable when several match. Claude-Session: https://claude.ai/code/session_01JTm9W2rNMjgzxqPVD6BGFi --- tests/behat/bootstrap/BehatCliContext.php | 27 ++++++++++++----------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/tests/behat/bootstrap/BehatCliContext.php b/tests/behat/bootstrap/BehatCliContext.php index 52bf281a..6e2ece10 100644 --- a/tests/behat/bootstrap/BehatCliContext.php +++ b/tests/behat/bootstrap/BehatCliContext.php @@ -221,17 +221,7 @@ public function fileMatchingShouldExist($pattern) #[Then('a file matching :pattern should contain:')] public function fileMatchingShouldContain(string $pattern, PyStringNode $text): void { - $matches = glob($this->workingDir . DIRECTORY_SEPARATOR . $pattern) ?: []; - Assert::assertNotEmpty($matches, sprintf('No file matching "%s" was found in the working directory.', $pattern)); - - $content = (string) file_get_contents($matches[0]); - $needle = trim((string) $text); - - if (str_contains($content, $needle)) { - return; - } - - throw new UnexpectedValueException(sprintf('File "%s" does not contain "%s".', $matches[0], $needle)); + $this->assertFileMatchingContains($pattern, $text, true); } /** @@ -239,18 +229,29 @@ public function fileMatchingShouldContain(string $pattern, PyStringNode $text): */ #[Then('a file matching :pattern should not contain:')] public function fileMatchingShouldNotContain(string $pattern, PyStringNode $text): void + { + $this->assertFileMatchingContains($pattern, $text, false); + } + + /** + * Asserts the first matching file does or does not contain a substring. + */ + protected function assertFileMatchingContains(string $pattern, PyStringNode $text, bool $shouldContain): void { $matches = glob($this->workingDir . DIRECTORY_SEPARATOR . $pattern) ?: []; Assert::assertNotEmpty($matches, sprintf('No file matching "%s" was found in the working directory.', $pattern)); + // Sort so the file inspected is deterministic when several patterns match. + sort($matches); $content = (string) file_get_contents($matches[0]); $needle = trim((string) $text); - if (!str_contains($content, $needle)) { + if (str_contains($content, $needle) === $shouldContain) { return; } - throw new UnexpectedValueException(sprintf('File "%s" unexpectedly contains "%s".', $matches[0], $needle)); + $message = $shouldContain ? 'File "%s" does not contain "%s".' : 'File "%s" unexpectedly contains "%s".'; + throw new UnexpectedValueException(sprintf($message, $matches[0], $needle)); } /** From 4f1aad96a4f4dc22a2658455b06baa01c8ce60d3 Mon Sep 17 00:00:00 2001 From: Alex Skrypnyk Date: Sun, 5 Jul 2026 14:08:03 +1000 Subject: [PATCH 3/4] Asserted JUnit report structure by parsing the XML rather than matching substrings. The renderer tests now load the output with SimpleXML and assert on '//failure' / '//testcase' counts and the 'tests' / 'failures' attributes, which also validates that the hand-built report is well-formed XML. Claude-Session: https://claude.ai/code/session_01JTm9W2rNMjgzxqPVD6BGFi --- tests/phpunit/src/AccessibilityTraitTest.php | 36 +++++++++++++------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/tests/phpunit/src/AccessibilityTraitTest.php b/tests/phpunit/src/AccessibilityTraitTest.php index e86bdefa..da1880da 100644 --- a/tests/phpunit/src/AccessibilityTraitTest.php +++ b/tests/phpunit/src/AccessibilityTraitTest.php @@ -350,13 +350,17 @@ public function testAggregateCaptureFormatsUrlsAndRecordsEntry(): void { public function testRenderJunitFailuresByThreshold(?string $threshold, int $expected_failures): void { $xml = $this->testObject->testRenderJunit(static::createJunitResults(), 'Feature', 'Scenario', $threshold); + $doc = simplexml_load_string($xml); + $this->assertInstanceOf(\SimpleXMLElement::class, $doc, 'The rendered report is well-formed XML.'); // Only violations meeting the effective threshold become cases. - $this->assertSame($expected_failures, substr_count($xml, ' elements reflects the threshold.'); + $this->assertCount($expected_failures, $doc->xpath('//failure') ?: []); // Every emitted testcase is counted, so tests stays 5 (3 violations + 2 - // passes) regardless of how many are failures versus advisory. - $this->assertStringContainsString(sprintf('', $expected_failures), $xml); - // The suite-level and file-level failure counts agree. - $this->assertSame(2, substr_count($xml, sprintf('failures="%d"', $expected_failures))); + // passes) whether a violation is a failure or advisory. + $this->assertCount(5, $doc->xpath('//testcase') ?: []); + $this->assertSame('5', (string) $doc['tests']); + // The file-level and suite-level failure counts agree. + $this->assertSame((string) $expected_failures, (string) $doc['failures']); + $this->assertSame((string) $expected_failures, (string) $doc->testsuite['failures']); } public static function dataProviderRenderJunitFailuresByThreshold(): array { @@ -373,11 +377,13 @@ public static function dataProviderRenderJunitFailuresByThreshold(): array { public function testRenderJunitWarningKeepsAdvisoryViolationsVisibleWithoutFailing(): void { $xml = $this->testObject->testRenderJunit(static::createJunitResults(), 'Feature', 'Scenario', 'never'); - // Advisory mode: no violation is serialised as a failure. + $doc = simplexml_load_string($xml); + $this->assertInstanceOf(\SimpleXMLElement::class, $doc); + // Advisory mode: no violation is serialised as a failure... + $this->assertCount(0, $doc->xpath('//failure') ?: []); $this->assertStringNotContainsString('assertStringContainsString('failures="0"', $xml); - // The findings stay visible as passing testcases carrying . - $this->assertStringContainsString('', $xml); + // ...but every finding stays visible as a passing testcase with . + $this->assertCount(3, $doc->xpath('//system-out') ?: []); $this->assertStringContainsString('classname="accessibility.image-alt"', $xml); $this->assertStringContainsString('classname="accessibility.link-name"', $xml); } @@ -399,10 +405,14 @@ public function testRenderJunitCountsEveryEmittedTestcase(): void { $xml = $this->testObject->testRenderJunit($results, 'Feature', 'Scenario', 'any'); - // One per affected node (2), plus three passing testcases: the - // tests and failures attributes count actual emitted elements. - $this->assertSame(2, substr_count($xml, 'assertInstanceOf(\SimpleXMLElement::class, $doc); + // One per affected node (2), plus three passing testcases: tests + // and failures count actual emitted elements, not violations. + $this->assertCount(2, $doc->xpath('//failure') ?: []); + $this->assertCount(5, $doc->xpath('//testcase') ?: []); + $this->assertSame('5', (string) $doc['tests']); + $this->assertSame('2', (string) $doc['failures']); } /** From 4d4405ff45b8f8160471d1ef7a09bb10fd9fc703 Mon Sep 17 00:00:00 2001 From: Alex Skrypnyk Date: Sun, 5 Jul 2026 14:29:18 +1000 Subject: [PATCH 4/4] Re-triggered CI to clear a flaky check.