Skip to content

fix: preserve gotest parser output for nested failures#31210

Open
immanuwell wants to merge 1 commit into
openshift:mainfrom
immanuwell:fix/junitreport-gotest-nested-output
Open

fix: preserve gotest parser output for nested failures#31210
immanuwell wants to merge 1 commit into
openshift:mainfrom
immanuwell:fix/junitreport-gotest-nested-output

Conversation

@immanuwell
Copy link
Copy Markdown

@immanuwell immanuwell commented May 25, 2026

This fixes a small junitreport parser bug in nested go test output.

What changed:

  • preserve trailing newlines in failure, skip, and stdout text
  • keep post-failure nested log lines attached to the right parent test

Repro on main before this patch:

go test ./tools/junitreport/pkg/parser/gotest

The existing fixtures fail there, including the nested output cases built from real go test logs.

Checked:

go test ./tools/junitreport/pkg/parser/gotest -run 'TestNestedParse/(nested_tests_with_inline_output|multi-suite_nested_output_with_coverage)' -v
go test ./tools/junitreport/...

Signed-off-by: immanuwell <pchpr.00@list.ru>
@openshift-merge-bot
Copy link
Copy Markdown
Contributor

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 openshift-ci Bot requested review from deads2k and p0lyn0mial May 25, 2026 13:41
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented May 25, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: immanuwell
Once this PR has been reviewed and has the lgtm label, please assign deads2k 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:

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

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 25, 2026

Walkthrough

This PR refines JUnit test report parsing in the go test output handler. It adds a utility to join output lines while preserving trailing newlines, applies this utility to failure messages and skip output, and corrects indentation depth tracking to properly attribute double-indented nested output to parent test cases.

Changes

JUnit Parser Output and Indentation Handling

Layer / File(s) Summary
Output assembly with trailing newline helper
tools/junitreport/pkg/parser/gotest/parser.go
Introduces joinLinesPreservingTrailingNewline(lines []string) string helper function that returns an empty string for empty slices and otherwise joins lines with newlines and appends a trailing newline. The helper is applied to failure output, skip messages, and system output assembly to replace strings.Join usage.
Double-indented nested output indentation tracking
tools/junitreport/pkg/parser/gotest/parser.go
Refines result-mode output-to-test attribution by capturing originalDepth and conditionally decrementing depth when the original depth targets a test case with failure or skip state. This corrects how subsequent output at that indentation level is attributed when a child test has closed and the parent has failure or skip messages.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

🚥 Pre-merge checks | ✅ 11 | ❌ 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 (11 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix: preserve gotest parser output for nested failures' directly and clearly summarizes the main change—preserving output in the gotest parser for nested test failures, which aligns with the PR objectives of fixing trailing newline drops and log association issues.
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.
Stable And Deterministic Test Names ✅ Passed PR contains no Ginkgo test definitions. All test files use standard Go testing.T patterns, not Ginkgo's It(), Describe(), Context(), or When() functions. The custom check is not applicable.
Test Structure And Quality ✅ Passed PR modifies source code (parser.go), not Ginkgo tests. Repository uses standard Go testing, not Ginkgo, so check is not applicable.
Microshift Test Compatibility ✅ Passed No Ginkgo e2e tests were added in this PR. The changes are to a parser utility tool (tools/junitreport/pkg/parser/gotest/parser.go), not new test definitions.
Single Node Openshift (Sno) Test Compatibility ✅ Passed This PR modifies a junitreport parser utility file (tools/junitreport/pkg/parser/gotest/parser.go) and does not add any new Ginkgo e2e tests. The custom check is not applicable.
Topology-Aware Scheduling Compatibility ✅ Passed PR modifies only junitreport parser utility (tools/junitreport), not deployment manifests, operator code, or controllers. Check doesn't apply.
Ote Binary Stdout Contract ✅ Passed The changed file is in junitreport, a standalone utility tool not part of openshift-tests/OTE framework. The OTE Binary Stdout Contract check does not apply to non-OTE binaries.
Ipv6 And Disconnected Network Test Compatibility ✅ Passed PR does not add any Ginkgo e2e tests. Changes are to tools/junitreport/pkg/parser/gotest/parser.go, a utility library for parsing Go test output, not e2e tests.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ 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.

@openshift-ci openshift-ci Bot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label May 25, 2026
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented May 25, 2026

Hi @immanuwell. Thanks for your PR.

I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@openshift-ci openshift-ci Bot added the ready-for-human-review Indicates a PR has been reviewed by automated tools and is ready for human review label May 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. ready-for-human-review Indicates a PR has been reviewed by automated tools and is ready for human review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant