wip: add support for parsing JUnit testcase lifecycle

[everettraven]

Summary by CodeRabbit

• New Features
  • JUnit test results now include lifecycle metadata for each test case.
  • Lifecycle information is preserved when test cases are processed, alongside status, duration, and failure details.

[openshift-merge-bot]

Pipeline controller notification
This repo is configured to use the pipeline controller. Second-stage tests will be triggered either automatically or after lgtm label is added, depending on the repository configuration. The pipeline controller will automatically detect which contexts are required and will utilize /test Prow commands to trigger the second stage.

For optional jobs, comment /test ? to see a list of all defined jobs. To trigger manually all jobs from second stage use /pipeline required command.

This repository is configured in: automatic mode

[openshift-ci]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[coderabbitai]

Walkthrough

JUnit test cases now carry lifecycle XML metadata, which is represented in extracted test case entries. The Prow loader propagates this value, and tests cover passing and failing cases while preserving existing fields.

Changes

JUnit lifecycle propagation

Layer / File(s)	Summary
Lifecycle data contracts pkg/apis/junit/types.go, pkg/dataloader/prowloader/types/types.go	Adds Lifecycle fields to JUnit TestCase and extracted TestCaseEntry structures.
Lifecycle extraction and validation pkg/dataloader/prowloader/prow.go, pkg/dataloader/prowloader/extract_test_cases_test.go	Copies lifecycle values during test case extraction and validates propagation for passing and failing cases.
Estimated code review effort: 2 (Simple)	~5 minutes

🚥 Pre-merge checks | ✅ 20 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Test Coverage For New Features	⚠️ Warning	Lifecycle is only unit-tested in extractTestCases; the final prowJobRunTestRow path drops it, so the new feature lacks end-to-end coverage.	Add a regression test for prowJobRunTestsFromGCS/buildJobRunResult that asserts Lifecycle survives into returned test rows, and wire the field through prowJobRunTestRow/testCols.

✅ Passed checks (20 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: adding JUnit testcase lifecycle support during parsing and propagation.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
Go Error Handling	✅ Passed	Touched code only adds lifecycle fields/tests; no ignored errors, panic use, or nil-deref risks were introduced.
Sql Injection Prevention	✅ Passed	PR only adds JUnit Lifecycle fields and propagates them; no SQL query construction changed, and existing DB writes use static statements/parameters.
Excessive Css In React Should Use Styles	✅ Passed	PR only changes Go JUnit parsing/loader files; no React components or inline style objects are present, so this check is not applicable.
Single Responsibility And Clear Naming	✅ Passed	The PR adds a single Lifecycle field with matching tests; the affected structs and method names stay focused and descriptive.
Feature Documentation	✅ Passed	PASS: docs/features has no JUnit/lifecycle feature doc to update; the only feature doc is unrelated job-analysis-symptoms, so docs don’t appear stale.
Stable And Deterministic Test Names	✅ Passed	No Ginkgo test titles were added; the new subtest names are static strings like "passing test with lifecycle".
Test Structure And Quality	✅ Passed	Not applicable: the PR adds table-driven unit tests, not Ginkgo, and they have no cluster ops, waits, or cleanup concerns.
Microshift Test Compatibility	✅ Passed	No new Ginkgo e2e tests or MicroShift-sensitive APIs are added; this PR only updates JUnit parsing and unit tests.
Single Node Openshift (Sno) Test Compatibility	✅ Passed	No new Ginkgo e2e tests were added; the only test change is a plain Go unit test, so the SNO compatibility check is not applicable.
Topology-Aware Scheduling Compatibility	✅ Passed	Only JUnit parsing/data-model/test code changed; no deployment manifests, controllers, or scheduling constraints were introduced.
Ote Binary Stdout Contract	✅ Passed	Touched files only add lifecycle fields/tests; no main/init/TestMain or stdout-print APIs appear in the modified code.
Ipv6 And Disconnected Network Test Compatibility	✅ Passed	No new Ginkgo e2e tests were added; the changed test is a unit test with no IPv4 or external-network assumptions.
No-Weak-Crypto	✅ Passed	Changed files only add JUnit lifecycle plumbing/tests; no crypto imports, algorithms, or secret comparisons appear in the diff or touched code.
Container-Privileges	✅ Passed	PR only changes Go types/tests; no Kubernetes/container manifests or privilege settings were added in touched files.
No-Sensitive-Data-In-Logs	✅ Passed	The diff only adds lifecycle plumbing and tests; no new log statements or expanded log fields were introduced.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: everettraven
Once this PR has been reviewed and has the lgtm label, please assign dgoodwin for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

pkg/dataloader/prowloader/prow.go (1)

1713-1727: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Preserve lifecycle metadata for duplicate test cases.

When repeated (suite, test) entries are collapsed into a flake, Lifecycle is retained only from the first occurrence. If an earlier retry lacks the attribute and a later retry provides it, the extracted entry silently loses the metadata. Define a conflict policy and at minimum fill an empty existing value from a later non-empty value, with a regression test.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/dataloader/prowloader/prow.go` around lines 1713 - 1727, Update the
duplicate test-case handling around testCases so that when an existing entry has
an empty Lifecycle and a later occurrence provides a non-empty value, the
existing entry adopts that value, including when the status is converted to
Flake. Define the empty-versus-non-empty precedence consistently with the
existing Output behavior, and add a regression test covering this retry
sequence.

🧹 Nitpick comments (1)

pkg/dataloader/prowloader/extract_test_cases_test.go (1)

147-183: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Add coverage for the XML boundary.

These tests populate Lifecycle directly, so they do not verify that xml:"lifecycle,attr" actually parses incoming JUnit XML. Add a fixture or unmarshaling test, plus a duplicate/flake case covering lifecycle preservation. As per coding guidelines, new or modified functionality should include test coverage.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/dataloader/prowloader/extract_test_cases_test.go` around lines 147 - 183,
Add XML unmarshaling coverage in the extract test cases tests so lifecycle
values are read from a JUnit XML lifecycle attribute rather than assigned
directly on TestCase objects. Include both a normal lifecycle case and a
duplicate/flake case, then verify the extracted entries preserve each lifecycle
value.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@pkg/dataloader/prowloader/prow.go`:
- Line 1720: Update prowJobRunTestRow construction in prowJobRunTestsFromGCS to
include the Lifecycle value from each TestCaseEntry, and extend the row contract
or struct as needed so the metadata is preserved in the loader results.

---

Outside diff comments:
In `@pkg/dataloader/prowloader/prow.go`:
- Around line 1713-1727: Update the duplicate test-case handling around
testCases so that when an existing entry has an empty Lifecycle and a later
occurrence provides a non-empty value, the existing entry adopts that value,
including when the status is converted to Flake. Define the
empty-versus-non-empty precedence consistently with the existing Output
behavior, and add a regression test covering this retry sequence.

---

Nitpick comments:
In `@pkg/dataloader/prowloader/extract_test_cases_test.go`:
- Around line 147-183: Add XML unmarshaling coverage in the extract test cases
tests so lifecycle values are read from a JUnit XML lifecycle attribute rather
than assigned directly on TestCase objects. Include both a normal lifecycle case
and a duplicate/flake case, then verify the extracted entries preserve each
lifecycle value.

🪄 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: Repository YAML (base), Central YAML (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: b7ef5cd8-3405-40d1-ab8f-d277c4797cec

📥 Commits

Reviewing files that changed from the base of the PR and between 0b3257f and 3633b44.

📒 Files selected for processing (4)

• pkg/apis/junit/types.go
• pkg/dataloader/prowloader/extract_test_cases_test.go
• pkg/dataloader/prowloader/prow.go
• pkg/dataloader/prowloader/types/types.go

[coderabbitai]

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Propagate Lifecycle into the returned test rows.

extractTestCases now stores the value in TestCaseEntry, but prowJobRunTestsFromGCS builds prowJobRunTestRow values without assigning Lifecycle at Lines 1676-1686. The metadata is therefore dropped before this loader returns its results; extend the row contract and copy the field there as well.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/dataloader/prowloader/prow.go` at line 1720, Update prowJobRunTestRow
construction in prowJobRunTestsFromGCS to include the Lifecycle value from each
TestCaseEntry, and extend the row contract or struct as needed so the metadata
is preserved in the loader results.