Skip to content

Have different names for testsuites in junit.#318

Open
rhopp wants to merge 1 commit into
redhat-appstudio:mainfrom
rhopp:fix/junit
Open

Have different names for testsuites in junit.#318
rhopp wants to merge 1 commit into
redhat-appstudio:mainfrom
rhopp:fix/junit

Conversation

@rhopp

@rhopp rhopp commented Mar 5, 2026

Copy link
Copy Markdown
Collaborator

Playwright's built-in JUnit reporter names test suites after file paths, which results in duplicate suite names in the generated XML. This causes problems when CI tools try to aggregate or display test results — suites with the same name get merged or become indistinguishable.

Changes

  • Custom JUnit reporter (src/reporters/junit-with-project.ts): Heavily inspired by Playwright's built-in JUnit reporter, but uses the top-level describe() block name combined with the project name as the suite name (e.g. TSSC E2E - chromium instead of tests/tssc/full_workflow.test.ts)
  • Renamed describe blocks in full_workflow.test.ts, component.test.ts, and gitopsResource.test.ts to produce clean, unique suite names
  • Tekton task updates: Added PLAYWRIGHT_JUNIT_SUITE_NAME (PipelineRun name) and PLAYWRIGHT_JUNIT_SUITE_ID (TaskRun name) env vars via Kubernetes Downward API to both tssc-e2e and tssc-ui tasks, so the top-level element carries meaningful identifiers

Summary by CodeRabbit

  • New Features

    • Implemented enhanced JUnit test reporting with project-related context embedded in test results.
    • Added Playwright JUnit metadata injection to integration test environment variables.
  • Tests

    • Updated test suite naming for improved clarity and consistency across end-to-end and UI test reporting.

@coderabbitai

coderabbitai Bot commented Mar 5, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

This PR enhances the TSSC E2E testing framework by injecting Playwright JUnit metadata environment variables into Tekton task steps, introducing a custom JUnit reporter that embeds project and context information into XML output, and updating test suite descriptions for clarity.

Changes

Cohort / File(s) Summary
Tekton Task Configuration
integration-tests/tasks/tssc-e2e.yaml, integration-tests/tasks/tssc-ui.yaml
Injected PLAYWRIGHT_JUNIT_SUITE_NAME and PLAYWRIGHT_JUNIT_SUITE_ID environment variables from Tekton pipelineRun/taskRun labels into test runtime, enabling metadata association with originating pipeline/task identifiers.
Custom Playwright Reporter Implementation
src/reporters/junit-with-project.ts
New JUnitWithProjectReporter class implements Playwright Reporter interface to generate JUnit XML with aggregated test metrics, per-test case details, error classification, ANSI stripping, and Tekton metadata context embedded in output.
Playwright Reporter Configuration
playwright.config.ts
Updated reporter configuration to use custom ./src/reporters/junit-with-project.ts instead of standard junit reporter, preserving JUNIT_OUTPUT_FILE environment variable handling.
Test Suite Naming Updates
tests/tssc/full_workflow.test.ts, tests/ui/component.test.ts, tests/ui/gitopsResource.test.ts
Renamed test suite descriptions for improved clarity and reporting consistency: "TSSC Complete Workflow" → "TSSC E2E", "Component UI Test Suite" → "TSSC UI - Component", "Gitops Resource UI Test Suite" → "TSSC UI Gitops".

Sequence Diagram(s)

sequenceDiagram
    actor Tekton
    participant TestTask as Tekton Task<br/>(tssc-e2e.yaml)
    participant PlaywrightRunner as Playwright<br/>Test Runner
    participant CustomReporter as JUnitWithProject<br/>Reporter
    participant FileSystem as File System
    
    Tekton->>TestTask: Inject PLAYWRIGHT_JUNIT_SUITE_NAME<br/>PLAYWRIGHT_JUNIT_SUITE_ID<br/>(from pipelineRun/taskRun labels)
    TestTask->>PlaywrightRunner: Execute with env vars
    PlaywrightRunner->>PlaywrightRunner: Run tests
    PlaywrightRunner->>CustomReporter: onStart(config, suite)
    PlaywrightRunner->>PlaywrightRunner: Process test results
    PlaywrightRunner->>CustomReporter: onEnd()
    CustomReporter->>CustomReporter: Aggregate per-project suites<br/>Build testcase entries<br/>Classify failures/errors<br/>Serialize to JUnit XML
    CustomReporter->>FileSystem: Write XML to junit.xml
    FileSystem-->>CustomReporter: Success
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • PR #114: Modifies Playwright/JUnit reporting and Tekton test task configs, directly aligned with reporter and metadata integration changes.
  • PR #210: Updates Playwright JUnit reporter configuration, with focus on output filename handling related to the custom reporter implementation.
  • PR #18: Adds tssc-e2e task and refactors testplan usage; this PR enhances that task with metadata injection and custom reporter wiring.

Suggested labels

testing, ui-testing, configuration, enhancement

Suggested reviewers

  • xinredhat
  • jsmid1
  • jkopriva
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is partially related to the changeset—it references renaming test suites in JUnit, which matches several file changes (test suite renaming), but it doesn't capture the main technical innovation: the custom JUnit reporter that uses top-level describe names and Playwright project names instead of file paths.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@rhopp rhopp marked this pull request as ready for review March 6, 2026 15:32
@qodo-code-review

Copy link
Copy Markdown

Review Summary by Qodo

Implement custom JUnit reporter with unique suite names
✨ Enhancement

Grey Divider

Walkthroughs

Description
• Implement custom JUnit reporter using describe block names
• Combine top-level describe names with project names for unique suite identifiers
• Rename test describe blocks for cleaner, more meaningful suite names
• Add Kubernetes Downward API environment variables to Tekton tasks
Diagram
flowchart LR
  A["Playwright Config"] -->|uses| B["Custom JUnit Reporter"]
  B -->|reads| C["Describe Block Names"]
  B -->|reads| D["Project Names"]
  B -->|reads| E["Env Variables"]
  C -->|combines with| D
  D -->|creates| F["Unique Suite Names"]
  E -->|populates| G["Root Testsuites Element"]
  H["Tekton Tasks"] -->|inject| E
  F -->|generates| I["JUnit XML"]
  G -->|generates| I
Loading

Grey Divider

File Changes

1. src/reporters/junit-with-project.ts ✨ Enhancement +317/-0

Custom JUnit reporter with project-aware suite naming

• New custom JUnit reporter implementation based on Playwright's built-in reporter
• Uses top-level describe block name combined with project name for suite naming
• Supports environment variables PLAYWRIGHT_JUNIT_SUITE_NAME and PLAYWRIGHT_JUNIT_SUITE_ID for
 root testsuites element
• Includes full XML serialization with ANSI escape stripping and error classification

src/reporters/junit-with-project.ts


2. playwright.config.ts ⚙️ Configuration changes +1/-1

Switch to custom JUnit reporter

• Replace built-in JUnit reporter with custom junit-with-project.ts reporter
• Maintains same output file configuration

playwright.config.ts


3. tests/tssc/full_workflow.test.ts ✨ Enhancement +1/-1

Rename describe block to TSSC E2E

• Rename top-level describe block from 'TSSC Complete Workflow' to 'TSSC E2E'
• Produces cleaner suite name when combined with project name

tests/tssc/full_workflow.test.ts


View more (4)
4. tests/ui/component.test.ts ✨ Enhancement +1/-1

Rename describe block to TSSC UI - Component

• Rename top-level describe block from 'Component UI Test Suite' to 'TSSC UI - Component'
• Improves suite name clarity and consistency

tests/ui/component.test.ts


5. tests/ui/gitopsResource.test.ts ✨ Enhancement +1/-1

Rename describe block to TSSC UI Gitops

• Rename top-level describe block from 'Gitops Resource UI Test Suite' to 'TSSC UI Gitops'
• Aligns naming convention with other test suites

tests/ui/gitopsResource.test.ts


6. integration-tests/tasks/tssc-e2e.yaml ⚙️ Configuration changes +8/-0

Add Kubernetes Downward API env vars for JUnit

• Add PLAYWRIGHT_JUNIT_SUITE_NAME environment variable via Kubernetes Downward API
• Add PLAYWRIGHT_JUNIT_SUITE_ID environment variable via Kubernetes Downward API
• Both variables reference PipelineRun and TaskRun labels for meaningful identifiers

integration-tests/tasks/tssc-e2e.yaml


7. integration-tests/tasks/tssc-ui.yaml ⚙️ Configuration changes +8/-0

Add Kubernetes Downward API env vars for JUnit

• Add PLAYWRIGHT_JUNIT_SUITE_NAME environment variable via Kubernetes Downward API
• Add PLAYWRIGHT_JUNIT_SUITE_ID environment variable via Kubernetes Downward API
• Both variables reference PipelineRun and TaskRun labels for meaningful identifiers

integration-tests/tasks/tssc-ui.yaml


Grey Divider

Qodo Logo

@qodo-code-review

qodo-code-review Bot commented Mar 6, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (3) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider


Action required

1. Totals not reset 🐞 Bug ⛯ Reliability
Description
The reporter’s aggregate counters are instance state that never reset in onBegin(), so repeated runs
in the same process (e.g., Playwright UI mode) can inflate the root <testsuites> counts. This can
lead CI/JUnit consumers to display incorrect totals.
Code

src/reporters/junit-with-project.ts[R37-54]

+  private totalTests = 0;
+  private totalFailures = 0;
+  private totalSkipped = 0;
+  private totalErrors = 0;
+  private stripANSIControlSequences = false;
+  private includeProjectInTestName = false;
+
+  constructor(options: JUnitOptions = {}) {
+    this.outputFile = options.outputFile || 'test-results/junit.xml';
+    this.stripANSIControlSequences = !!options.stripANSIControlSequences;
+    this.includeProjectInTestName = !!options.includeProjectInTestName;
+  }
+
+  onBegin(config: FullConfig, suite: Suite): void {
+    this.config = config;
+    this.suite = suite;
+    this.timestamp = new Date();
+  }
Evidence
total* fields are initialized once and incremented per suite, but onBegin() does not reset them;
these accumulated values are emitted into the root <testsuites> attributes.

src/reporters/junit-with-project.ts[37-54]
src/reporters/junit-with-project.ts[108-112]
src/reporters/junit-with-project.ts[64-74]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The custom JUnit reporter keeps aggregate counters as instance fields and increments them during a run, but does not reset them in `onBegin()`. If Playwright reuses the reporter instance across multiple runs (e.g., UI mode), the root `&lt;testsuites&gt;` counts will accumulate and become incorrect.

### Issue Context
Counters are declared as fields and used to populate the root element attributes.

### Fix Focus Areas
- src/reporters/junit-with-project.ts[37-54]
- src/reporters/junit-with-project.ts[108-112]
- src/reporters/junit-with-project.ts[64-74]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. ANSI option ignored 🐞 Bug ✓ Correctness
Description
stripANSIControlSequences is exposed as an option but ANSI codes are stripped unconditionally when
building failure/error payloads and while classifying errors. This makes the option effectively
non-functional and can reduce debugging fidelity when ANSI output was desired.
Code

src/reporters/junit-with-project.ts[R178-200]

+    let classification: 'failure' | 'error' | null = null;
+    if (!test.ok()) {
+      const errorInfo = classifyError(test);
+      if (errorInfo) {
+        classification = errorInfo.elementName;
+        entry.children!.push({
+          name: errorInfo.elementName,
+          attributes: {
+            message: errorInfo.message,
+            type: errorInfo.type,
+          },
+          text: stripAnsiEscapes(formatFailure(test)),
+        });
+      } else {
+        classification = 'failure';
+        entry.children!.push({
+          name: 'failure',
+          attributes: {
+            message: `${path.basename(test.location.file)}:${test.location.line}:${test.location.column} ${test.title}`,
+            type: 'FAILURE',
+          },
+          text: stripAnsiEscapes(formatFailure(test)),
+        });
Evidence
Although stripANSIControlSequences is stored and later passed into XML escaping, ANSI stripping is
applied before serialization regardless of the option, so it cannot be disabled.

src/reporters/junit-with-project.ts[44-48]
src/reporters/junit-with-project.ts[178-200]
src/reporters/junit-with-project.ts[239-246]
src/reporters/junit-with-project.ts[302-305]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`stripANSIControlSequences` is presented as a configurable option, but ANSI escape codes are removed unconditionally in multiple places (failure text and error classification). This makes the option ineffective.

### Issue Context
ANSI stripping happens before XML escaping/serialization, so it cannot be controlled by the `stripANSI` argument passed into `escape()`.

### Fix Focus Areas
- src/reporters/junit-with-project.ts[44-48]
- src/reporters/junit-with-project.ts[178-200]
- src/reporters/junit-with-project.ts[239-246]
- src/reporters/junit-with-project.ts[302-305]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Testcase IDs may collide 🐞 Bug ✧ Quality
Description
Suite names now include the project, but testcase identifiers do not by default: testcase.name
removes the project segment and testcase.classname is the file-suite title. In multi-project runs
this can still produce identical testcase (classname,name) pairs across projects for some JUnit
consumers.
Code

src/reporters/junit-with-project.ts[R140-148]

+  private async _addTestCase(suiteName: string, namePrefix: string, test: TestCase, entries: XMLEntry[]): Promise<'failure' | 'error' | null> {
+    const entry: XMLEntry = {
+      name: 'testcase',
+      attributes: {
+        // Skip root, project, file
+        name: namePrefix + test.titlePath().slice(3).join(' › '),
+        // filename
+        classname: suiteName,
+        time: test.results.reduce((acc, value) => acc + value.duration, 0) / 1000,
Evidence
The reporter supports multi-project runs (config generates many e2e-/ui- projects) but does not
enable includeProjectInTestName, and classname is derived from the file suite title passed from
_buildTestSuite(). This leaves testcase identifiers identical across projects for the same
file/test.

playwright.config.ts[160-193]
playwright.config.ts[212-217]
src/reporters/junit-with-project.ts[92-103]
src/reporters/junit-with-project.ts[140-148]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
Even though `&lt;testsuite name&gt;` now includes the project, `&lt;testcase&gt;` identifiers can still be identical across projects because `testcase.name` strips the project segment by default and `testcase.classname` is based on the file suite title.

### Issue Context
This repo commonly runs the same test files across multiple Playwright projects (e2e-* and ui-*). Some JUnit consumers deduplicate/merge by `(classname, name)`.

### Fix Focus Areas
- playwright.config.ts[212-217]
- src/reporters/junit-with-project.ts[92-103]
- src/reporters/junit-with-project.ts[140-148]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

@rhopp

rhopp commented Mar 6, 2026

Copy link
Copy Markdown
Collaborator Author

/retest

Comment on lines +37 to +54
private totalTests = 0;
private totalFailures = 0;
private totalSkipped = 0;
private totalErrors = 0;
private stripANSIControlSequences = false;
private includeProjectInTestName = false;

constructor(options: JUnitOptions = {}) {
this.outputFile = options.outputFile || 'test-results/junit.xml';
this.stripANSIControlSequences = !!options.stripANSIControlSequences;
this.includeProjectInTestName = !!options.includeProjectInTestName;
}

onBegin(config: FullConfig, suite: Suite): void {
this.config = config;
this.suite = suite;
this.timestamp = new Date();
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. Totals not reset 🐞 Bug ⛯ Reliability

The reporter’s aggregate counters are instance state that never reset in onBegin(), so repeated runs
in the same process (e.g., Playwright UI mode) can inflate the root <testsuites> counts. This can
lead CI/JUnit consumers to display incorrect totals.
Agent Prompt
### Issue description
The custom JUnit reporter keeps aggregate counters as instance fields and increments them during a run, but does not reset them in `onBegin()`. If Playwright reuses the reporter instance across multiple runs (e.g., UI mode), the root `<testsuites>` counts will accumulate and become incorrect.

### Issue Context
Counters are declared as fields and used to populate the root element attributes.

### Fix Focus Areas
- src/reporters/junit-with-project.ts[37-54]
- src/reporters/junit-with-project.ts[108-112]
- src/reporters/junit-with-project.ts[64-74]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

@konflux-ci-qe-bot

Copy link
Copy Markdown

@rhopp: The following test has Failed, say /retest to rerun failed tests.

PipelineRun Name Status Rerun command Build Log Test Log
e2e-4.20-wn5lx Failed /retest View Pipeline Log View Test Logs

Inspecting Test Artifacts

To inspect your test artifacts, follow these steps:

  1. Install ORAS (see the ORAS installation guide).
  2. Download artifacts with the following commands:
mkdir -p oras-artifacts
cd oras-artifacts
oras pull quay.io/konflux-test-storage/rhtap-team/rhtap-cli:e2e-4.20-wn5lx

Test results analysis

<not enabled>

OCI Artifact Browser URL

<not enabled>

…ect names

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@rhopp rhopp changed the title WIP: Have different names for testsuites in junit. Have different names for testsuites in junit. Apr 28, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/reporters/junit-with-project.ts`:
- Around line 58-60: The reporter currently groups tests per file by iterating
projectSuite.suites (fileSuite) and calling _buildTestSuite once per file, which
names the testsuite using suite.suites[0].title and causes multiple top-level
describe() blocks in the same file to be merged under the first name; change the
logic to split suites by top-level describe blocks: for each projectSuite
(project) iterate each fileSuite and then iterate fileSuite.suites (each
top-level describe) and call _buildTestSuite for each top-level describe suite
(passing the correct titles/parents) so each top-level describe becomes its own
<testsuite>; apply the same change to the similar loop/logic referenced in the
95-120 region to ensure consistent splitting and accurate suite names.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: f6092633-a02b-4e28-a918-2d07619e05bf

📥 Commits

Reviewing files that changed from the base of the PR and between d9c32bb and 9c6b9a4.

📒 Files selected for processing (7)
  • integration-tests/tasks/tssc-e2e.yaml
  • integration-tests/tasks/tssc-ui.yaml
  • playwright.config.ts
  • src/reporters/junit-with-project.ts
  • tests/tssc/full_workflow.test.ts
  • tests/ui/component.test.ts
  • tests/ui/gitopsResource.test.ts

Comment on lines +58 to +60
for (const projectSuite of this.suite.suites) {
for (const fileSuite of projectSuite.suites)
children.push(await this._buildTestSuite(projectSuite.title, fileSuite));

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Split JUnit suites by top-level describe, not by file.

Right now the reporter builds one <testsuite> per file and then names it from suite.suites[0].title. If a file has multiple top-level test.describe() blocks, every testcase in that file is reported under the first block’s name, so suite names become inaccurate and can still collide in CI.

Suggested fix
   async onEnd(result: FullResult): Promise<void> {
     const children: XMLEntry[] = [];
     for (const projectSuite of this.suite.suites) {
-      for (const fileSuite of projectSuite.suites)
-        children.push(await this._buildTestSuite(projectSuite.title, fileSuite));
+      for (const fileSuite of projectSuite.suites) {
+        const suitesToReport = fileSuite.suites.length ? fileSuite.suites : [fileSuite];
+        for (const topLevelSuite of suitesToReport)
+          children.push(await this._buildTestSuite(projectSuite.title, topLevelSuite));
+      }
     }
@@
-    const topLevelDescribe = suite.suites.length > 0 ? suite.suites[0].title : '';
-    const suiteLabel = topLevelDescribe || suite.title;
+    const suiteLabel = suite.title;
     const suiteName = projectName
       ? `${suiteLabel} - ${projectName}`
       : suiteLabel;

Also applies to: 95-120

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/reporters/junit-with-project.ts` around lines 58 - 60, The reporter
currently groups tests per file by iterating projectSuite.suites (fileSuite) and
calling _buildTestSuite once per file, which names the testsuite using
suite.suites[0].title and causes multiple top-level describe() blocks in the
same file to be merged under the first name; change the logic to split suites by
top-level describe blocks: for each projectSuite (project) iterate each
fileSuite and then iterate fileSuite.suites (each top-level describe) and call
_buildTestSuite for each top-level describe suite (passing the correct
titles/parents) so each top-level describe becomes its own <testsuite>; apply
the same change to the similar loop/logic referenced in the 95-120 region to
ensure consistent splitting and accurate suite names.

@konflux-ci-qe-bot

Copy link
Copy Markdown

Scenario: pr-e2e-tests
@rhopp: The following test has Failed, say /retest to rerun failed tests.

PipelineRun Name Status Rerun command Build Log Test Log
e2e-4.20-pdqwv Failed /retest View Pipeline Log View Test Logs

Inspecting Test Artifacts

To inspect your test artifacts, follow these steps:

  1. Install ORAS (see the ORAS installation guide).
  2. Download artifacts with the following commands:
mkdir -p oras-artifacts
cd oras-artifacts
oras pull quay.io/konflux-test-storage/rhtap-team/rhtap-cli:e2e-4.20-pdqwv

Test results analysis

<not enabled>

OCI Artifact Browser URL

<not enabled>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants