Skip to content

Commit 36ce75a

Browse files
rickystewartrickystewart
authored
Merge pull request #158825 from rickystewart/backport24.3-158824
release-24.3: ci: ensure we correctly report owners for generated tests
2 parents a72b17e + 4125137 commit 36ce75a

File tree

6 files changed

+87
-13
lines changed

6 files changed

+87
-13
lines changed

‎.github/workflows/github-actions-essential-ci.yml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,11 @@ jobs:
253253
- run: ./build/github/prepare-summarize-build.sh
254254
- name: run tests
255255
run: ./build/github/unit-tests.sh
256+
- name: generate code
257+
if: failure()
258+
# NB: To correctly report test owners, we'll have to make sure all Go
259+
# code is generated and hoisted into the workspace (#107885).
260+
run: ./build/github/generate-go-code.sh
256261
- name: upload test results
257262
run: ./build/github/summarize-build.sh bes.bin
258263
if: always()

‎build/github/generate-go-code.sh‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
#!/usr/bin/env bash
2+
3+
# Copyright 2025 The Cockroach Authors.
4+
#
5+
# Use of this software is governed by the CockroachDB Software License
6+
# included in the /LICENSE file.
7+
8+
set -euo pipefail
9+
10+
ENGFLOW_ARGS="--config crosslinux --jobs 50 $(./build/github/engflow-args.sh) --remote_download_minimal"
11+
12+
bazel run //pkg/gen:code $ENGFLOW_ARGS

‎pkg/build/util/util.go‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,18 @@ type XMLMessage struct {
5656
Contents string `xml:",chardata"`
5757
}
5858

59+
// AnyFailures returns true iff there are any errors/failures in the test.xml.
60+
func AnyFailures(suites TestSuites) bool {
61+
for _, suite := range suites.Suites {
62+
for _, testCase := range suite.TestCases {
63+
if testCase.Failure != nil || testCase.Error != nil {
64+
return true
65+
}
66+
}
67+
}
68+
return false
69+
}
70+
5971
// OutputOfBinaryRule returns the path of the binary produced by the
6072
// given build target, relative to bazel-bin. That is,
6173
//

‎pkg/build/util/util_test.go‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,48 @@ import (
1313
"github.com/stretchr/testify/require"
1414
)
1515

16+
func TestAnyFailures(t *testing.T) {
17+
xml1 := `<testsuites>
18+
<testsuite errors="0" failures="0" skipped="0" tests="1" time="0.000" name="github.com/cockroachdb/cockroach/pkg/build/util.TestMergeXml" timestamp="2025-12-02T20:59:24.432Z">
19+
<testcase classname="util" name="TestMergeXml" time="0.000"></testcase>
20+
</testsuite>
21+
<testsuite errors="0" failures="0" skipped="0" tests="1" time="0.000" name="github.com/cockroachdb/cockroach/pkg/build/util.TestMungeTestXML" timestamp="2025-12-02T20:59:24.433Z">
22+
<testcase classname="util" name="TestMungeTestXML" time="0.000"></testcase>
23+
</testsuite>
24+
<testsuite errors="0" failures="0" skipped="0" tests="1" time="0.000" name="github.com/cockroachdb/cockroach/pkg/build/util.TestOutputOfBinaryRule" timestamp="2025-12-02T20:59:24.432Z">
25+
<testcase classname="util" name="TestOutputOfBinaryRule" time="0.000"></testcase>
26+
</testsuite>
27+
<testsuite errors="0" failures="0" skipped="0" tests="1" time="0.000" name="github.com/cockroachdb/cockroach/pkg/build/util.TestOutputsOfGenrule" timestamp="2025-12-02T20:59:24.432Z">
28+
<testcase classname="util" name="TestOutputsOfGenrule" time="0.000"></testcase>
29+
</testsuite>
30+
</testsuites>`
31+
xml2 := `<testsuites>
32+
<testsuite errors="0" failures="1" skipped="0" tests="1" time="0.000" name="github.com/cockroachdb/cockroach/pkg/build/util.TestAnyFailures" timestamp="2025-12-02T21:01:33.393Z">
33+
<testcase classname="util" name="TestAnyFailures" time="0.000">
34+
<failure message="Failed" type="">=== RUN TestAnyFailures&#xA; util_test.go:33: &#xA; &#x9;Error Trace:&#x9;pkg/build/util/util_test.go:33&#xA; &#x9;Error: &#x9;An error is expected but got nil.&#xA; &#x9;Test: &#x9;TestAnyFailures&#xA;--- FAIL: TestAnyFailures (0.00s)&#xA;</failure>
35+
</testcase>
36+
</testsuite>
37+
<testsuite errors="0" failures="0" skipped="0" tests="1" time="0.000" name="github.com/cockroachdb/cockroach/pkg/build/util.TestMergeXml" timestamp="2025-12-02T21:01:33.393Z">
38+
<testcase classname="util" name="TestMergeXml" time="0.000"></testcase>
39+
</testsuite>
40+
<testsuite errors="0" failures="0" skipped="0" tests="1" time="0.000" name="github.com/cockroachdb/cockroach/pkg/build/util.TestMungeTestXML" timestamp="2025-12-02T21:01:33.393Z">
41+
<testcase classname="util" name="TestMungeTestXML" time="0.000"></testcase>
42+
</testsuite>
43+
<testsuite errors="0" failures="0" skipped="0" tests="1" time="0.000" name="github.com/cockroachdb/cockroach/pkg/build/util.TestOutputOfBinaryRule" timestamp="2025-12-02T21:01:33.393Z">
44+
<testcase classname="util" name="TestOutputOfBinaryRule" time="0.000"></testcase>
45+
</testsuite>
46+
<testsuite errors="0" failures="0" skipped="0" tests="1" time="0.000" name="github.com/cockroachdb/cockroach/pkg/build/util.TestOutputsOfGenrule" timestamp="2025-12-02T21:01:33.393Z">
47+
<testcase classname="util" name="TestOutputsOfGenrule" time="0.000"></testcase>
48+
</testsuite>
49+
</testsuites>`
50+
51+
var suite1, suite2 TestSuites
52+
require.NoError(t, xml.Unmarshal([]byte(xml1), &suite1))
53+
require.False(t, AnyFailures(suite1))
54+
require.NoError(t, xml.Unmarshal([]byte(xml2), &suite2))
55+
require.True(t, AnyFailures(suite2))
56+
}
57+
1658
func TestOutputOfBinaryRule(t *testing.T) {
1759
require.Equal(t, OutputOfBinaryRule("//pkg/cmd/cockroach-short", false),
1860
"pkg/cmd/cockroach-short/cockroach-short_/cockroach-short")

‎pkg/cmd/bazci/bazci.go‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -475,6 +475,9 @@ func removeEmergencyBallasts() {
475475
}
476476

477477
func processTestXmls(testXmls []string) error {
478+
// If any tests failed, we'll have to make sure that all generated code
479+
// exists in the workspace (see #107885).
480+
var generateCode sync.Once
478481
if doPost() {
479482
var postErrors []string
480483
for _, testXml := range testXmls {
@@ -489,6 +492,17 @@ func processTestXmls(testXmls []string) error {
489492
postErrors = append(postErrors, fmt.Sprintf("Failed to parse test.xml file with the following error: %+v", err))
490493
continue
491494
}
495+
if bazelutil.AnyFailures(testSuites) {
496+
generateCode.Do(func() {
497+
genCmd := exec.Command("bazel", "run", "//pkg/gen:code")
498+
genCmd.Stdout = os.Stdout
499+
genCmd.Stderr = os.Stderr
500+
err := genCmd.Run()
501+
if err != nil {
502+
fmt.Printf("got error %+v from bazel run //pkg/gen:code; continuing", err)
503+
}
504+
})
505+
}
492506
if err := githubpost.PostFromTestXMLWithFormatterName(githubPostFormatterName, testSuites); err != nil {
493507
postErrors = append(postErrors, fmt.Sprintf("Failed to process %s with the following error: %+v", testXml, err))
494508
continue

‎pkg/internal/codeowners/codeowners.go‎

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -169,19 +169,8 @@ func (co *CodeOwners) GetTestOwner(
169169
panic("test-eng team could not be found in TEAMS.yaml")
170170
}
171171

172-
// Workaround for #107885.
173-
if strings.Contains(packageName, "backup") {
174-
dr := co.GetTeamForAlias("cockroachdb/disaster-recovery")
175-
if dr.Name() == "" {
176-
panic("disaster-recovery team could not be found in TEAMS.yaml")
177-
}
178-
179-
_logs = append(_logs, fmt.Sprintf("assigning %s.%s to 'disaster-recovery' due to #107885", packageName, testName))
180-
_teams = []team.Team{dr}
181-
} else {
182-
_logs = append(_logs, fmt.Sprintf("assigning %s.%s to 'test-eng' as catch-all", packageName, testName))
183-
_teams = []team.Team{testEng}
184-
}
172+
_logs = append(_logs, fmt.Sprintf("assigning %s.%s to 'test-eng' as catch-all", packageName, testName))
173+
_teams = []team.Team{testEng}
185174
}
186175
return
187176
}

0 commit comments

Comments
 (0)