Skip to content

feat: add gotest-junit tester #730

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Draft
jpculp wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

feat: add gotest-junit tester #730

jpculp wants to merge 1 commit into from

Conversation

@jpculp
Copy link
Contributor

@jpculp jpculp commented Jan 9, 2026

Description of changes:

This tester executes Go tests such as neuron.test and nvidia.test in a similar fashion to kubetest2-tester-exec, but with the added benefit of outputting a JUnit XML file consistent with the ginkgov1 tester.

Testing of changes:

Ran the neuron.test using the gotest-junit tester.

Console output 
2026/01/08 22:22:45 [INFO] Total Nodes: 2
2026/01/08 22:22:45 [INFO] Total Neuron Count: 2, Neuron Per Node: 1
2026/01/08 22:22:45 [INFO] Total Neuron Core Count: 4, Neuron Core Per Node: 2
2026/01/08 22:22:45 [INFO] Total EFA Count: 0, EFA Per Node: 0
=== RUN   TestNeuronNodes
=== RUN   TestNeuronNodes/single-node
    neuron_test.go:60: Applying single node manifest
    neuron_test.go:65: Manifest applied successfully
=== RUN   TestNeuronNodes/single-node/Single_node_test_Job_succeeds
    neuron_test.go:72: Waiting for single node job to complete
=== NAME  TestNeuronNodes/single-node
    neuron_test.go:89: Test log for neuronx-single-node:

...

=== RUN   TestNeuronNodes/multi-node
    neuron_test.go:157: Skipping feature: "multi-node": name matched
--- PASS: TestNeuronNodes (355.36s)
    --- PASS: TestNeuronNodes/single-node (355.36s)
        --- PASS: TestNeuronNodes/single-node/Single_node_test_Job_succeeds (355.09s)
    --- SKIP: TestNeuronNodes/multi-node (0.00s)
PASS
JUnit XML output 
<testsuites>
        <testsuite name="neuron" tests="4" failures="0" errors="0" id="0" hostname="XXXXXXXX" skipped="1" time="1065.810" timestamp="2026-01-08T22:28:41Z">
                <testcase name="TestNeuronNodes" classname="" time="355.360"></testcase>
                        <system-out><![CDATA[    neuron_test.go:60: Applying single node manifest
    neuron_test.go:65: Manifest applied successfully
    neuron_test.go:89: Test log for neuronx-single-node:

...

        ]]></system-out>
                </testcase>
                <testcase name="TestNeuronNodes/single-node/Single_node_test_Job_succeeds" classname="" time="355.090">
                        <system-out><![CDATA[    neuron_test.go:72: Waiting for single node job to complete]]></system-out>
                </testcase>
                <testcase name="TestNeuronNodes/multi-node" classname="" time="0.000">
                        <skipped message="Skipped"><![CDATA[    neuron_test.go:157: Skipping feature: "multi-node": name matched]]></skipped>
                </testcase>
                <system-out><![CDATA[2026/01/08 22:22:45 [INFO] Total Nodes: 2
2026/01/08 22:22:45 [INFO] Total Neuron Count: 2, Neuron Per Node: 1
2026/01/08 22:22:45 [INFO] Total Neuron Core Count: 4, Neuron Core Per Node: 2
2026/01/08 22:22:45 [INFO] Total EFA Count: 0, EFA Per Node: 0]]></system-out>
        </testsuite>
</testsuites>

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@jpculp
feat: add gotest-junit tester
a578e8f 
This tester executes Go tests such as `neuron.test` and `nvidia.test` in a
similar fashion to `kubetest2-tester-exec`, but with the added benefit of
outputting a JUnit XML file consistent with the `ginkgov1` tester.

Signed-off-by: Patrick J.P. Culp <jpculp@amazon.com>
@jpculp jpculp requested review from mselim00 and yeazelm January 9, 2026 00:00
yeazelm
yeazelm reviewed Jan 9, 2026
View reviewed changes
internal/testers/gotestjunit/exec.go
This tester executes a Go test binary and generates JUnit XML output.
`

func (t *Tester) Execute() error {
Copy link
Contributor

@yeazelm yeazelm Jan 9, 2026

Choose a reason for hiding this comment

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

Is the intention to export these? Do we expect things outside this command to use them/

Copy link
Contributor Author

@jpculp jpculp Jan 9, 2026

Choose a reason for hiding this comment

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

I don't think so, but it's consistent with the ginkgov1 code.

internal/testers/gotestjunit/exec.go
Comment on lines +88 to +90
signals := make(chan os.Signal, 5)
signal.Notify(signals)
defer signal.Stop(signals)
Copy link
Contributor

@yeazelm yeazelm Jan 9, 2026

Choose a reason for hiding this comment

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

Is there a reason we are doing this with channels and having to manage signal handling vs just waiting on the binary in the main thread?

Copy link
Contributor Author

@jpculp jpculp Jan 9, 2026

Choose a reason for hiding this comment

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

We could probably just simplify it to testErr := cmd.Run(), but the current implementation is more consistent with the upstream kubetest2 exec.go.

jpculp
jpculp commented Jan 9, 2026
View reviewed changes
internal/testers/gotestjunit/exec.go
Comment on lines +88 to +90
signals := make(chan os.Signal, 5)
signal.Notify(signals)
defer signal.Stop(signals)
Copy link
Contributor Author

@jpculp jpculp Jan 9, 2026

Choose a reason for hiding this comment

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

We could probably just simplify it to testErr := cmd.Run(), but the current implementation is more consistent with the upstream kubetest2 exec.go.

internal/testers/gotestjunit/exec.go
Comment on lines +85 to +86
cmd.Stdout = io.MultiWriter(os.Stdout, &buf)
cmd.Stderr = io.MultiWriter(os.Stderr, &buf)
Copy link
Contributor Author

@jpculp jpculp Jan 9, 2026

Choose a reason for hiding this comment

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

Suggested change
cmd.Stdout = io.MultiWriter(os.Stdout, &buf)
cmd.Stderr = io.MultiWriter(os.Stderr, &buf)
// ensure we also capture output
var systemout bytes.Buffer
syncSystemOut := &mutexWriter{
writer: &systemout,
}
cmd.Stdout = io.MultiWriter(syncSystemOut, os.Stdout)
cmd.Stderr = io.MultiWriter(syncSystemOut, os.Stderr)

I wonder if we should add this to be more consistent with upstream kubetest2's junitexec.go.

internal/testers/gotestjunit/exec.go
This tester executes a Go test binary and generates JUnit XML output.
`

func (t *Tester) Execute() error {
Copy link
Contributor Author

@jpculp jpculp Jan 9, 2026

Choose a reason for hiding this comment

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

I don't think so, but it's consistent with the ginkgov1 code.

@jpculp jpculp marked this pull request as draft January 9, 2026 19:42
@jpculp
Copy link
Contributor Author

jpculp commented Jan 9, 2026

Dropping to draft while I work on attribution block(s).

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

Reviewers

@yeazelm yeazelm yeazelm left review comments

@mselim00 mselim00 Awaiting requested review from mselim00

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jpculp @yeazelm