Skip to content

Commit 919c521

Browse files
Merge extended test suite JUnit files to avoid duplicate skips
Jenkins JUnit does not handle skips in different files that have the same name, leading to invalid results. Merge all the JUnit output files into one by suite and test name. Failures override Successes, both override Skips.
1 parent d076366 commit 919c521

File tree

5 files changed

+183
-2
lines changed

5 files changed

+183
-2
lines changed

test/extended/conformance.sh

+2
Original file line numberDiff line numberDiff line change
@@ -29,4 +29,6 @@ TEST_PARALLEL="${PARALLEL_NODES:-5}" FOCUS="${pf}" SKIP="${ps}" TEST_REPORT_FILE
2929
os::log::info "Running serial tests"
3030
FOCUS="${sf}" SKIP="${ss}" TEST_REPORT_FILE_NAME=conformance_serial os::test::extended::run -- -ginkgo.noColor -ginkgo.v -test.timeout 2h ${TEST_EXTENDED_ARGS-} || exitstatus=$?
3131

32+
os::test::extended::merge_junit
33+
3234
exit $exitstatus

test/extended/core.sh

+2
Original file line numberDiff line numberDiff line change
@@ -34,4 +34,6 @@ os::log::info ""
3434
os::log::info "Running serial tests"
3535
FOCUS="${sf}" SKIP="${ss}" TEST_REPORT_FILE_NAME=core_serial os::test::extended::run -- -ginkgo.noColor -ginkgo.v -test.timeout 2h ${TEST_EXTENDED_ARGS-} || exitstatus=$?
3636

37+
os::test::extended::merge_junit
38+
3739
exit $exitstatus

test/extended/setup.sh

+14-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
# This abstracts starting up an extended server.
44

55
# If invoked with arguments, executes the test directly.
6-
function os::test::extended::focus {
6+
function os::test::extended::focus () {
77
if [[ $# -ne 0 ]]; then
88
os::log::info "Running custom: $*"
99
os::test::extended::test_list "$@"
@@ -27,6 +27,7 @@ function os::test::extended::setup () {
2727
os::util::ensure::built_binary_exists 'openshift'
2828
os::util::ensure::built_binary_exists 'oadm'
2929
os::util::ensure::built_binary_exists 'oc'
30+
os::util::ensure::built_binary_exists 'junitmerge' 'tools/junitmerge'
3031

3132
# ensure proper relative directories are set
3233
export EXTENDED_TEST_PATH="${OS_ROOT}/test/extended"
@@ -244,6 +245,18 @@ function os::test::extended::test_list () {
244245
}
245246
readonly -f os::test::extended::test_list
246247

248+
# Merge all of the JUnit output files in the TEST_REPORT_DIR into a single file.
249+
# This works around a gap in Jenkins JUnit reporter output that double counts skipped
250+
# files until https://github.com/jenkinsci/junit-plugin/pull/54 is merged.
251+
function os::test::extended::merge_junit () {
252+
local output
253+
output="$( mktemp )"
254+
"$( os::util::find::built_binary junitmerge )" "${TEST_REPORT_DIR}"/*.xml > "${output}"
255+
rm "${TEST_REPORT_DIR}"/*.xml
256+
mv "${output}" "${TEST_REPORT_DIR}/junit.xml"
257+
}
258+
readonly -f os::test::extended::merge_junit
259+
247260
# Not run by any suite
248261
readonly EXCLUDED_TESTS=(
249262
"\[Skipped\]"

tools/junitmerge/junitmerge.go

+155
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,155 @@
1+
package main
2+
3+
import (
4+
"encoding/xml"
5+
"log"
6+
"os"
7+
8+
"fmt"
9+
"github.com/openshift/origin/tools/junitreport/pkg/api"
10+
"sort"
11+
)
12+
13+
type uniqueSuites map[string]*suiteRuns
14+
15+
func (s uniqueSuites) Merge(namePrefix string, suite *api.TestSuite) {
16+
name := suite.Name
17+
if len(namePrefix) > 0 {
18+
name = namePrefix + "/"
19+
}
20+
existing, ok := s[name]
21+
if !ok {
22+
existing = newSuiteRuns(suite)
23+
s[name] = existing
24+
}
25+
26+
existing.Merge(suite.TestCases)
27+
28+
for _, suite := range suite.Children {
29+
s.Merge(name, suite)
30+
}
31+
}
32+
33+
type suiteRuns struct {
34+
suite *api.TestSuite
35+
runs map[string]*api.TestCase
36+
}
37+
38+
func newSuiteRuns(suite *api.TestSuite) *suiteRuns {
39+
return &suiteRuns{
40+
suite: suite,
41+
runs: make(map[string]*api.TestCase),
42+
}
43+
}
44+
45+
func (r *suiteRuns) Merge(testCases []*api.TestCase) {
46+
for _, testCase := range testCases {
47+
existing, ok := r.runs[testCase.Name]
48+
if !ok {
49+
r.runs[testCase.Name] = testCase
50+
continue
51+
}
52+
switch {
53+
case testCase.SkipMessage != nil:
54+
// if the new test is a skip, ignore it
55+
case existing.SkipMessage != nil && testCase.SkipMessage == nil:
56+
// always replace a skip with a non-skip
57+
r.runs[testCase.Name] = testCase
58+
case existing.FailureOutput == nil && testCase.FailureOutput != nil:
59+
// replace a passing test with a failing test
60+
r.runs[testCase.Name] = testCase
61+
}
62+
}
63+
}
64+
65+
func main() {
66+
log.SetFlags(0)
67+
suites := make(uniqueSuites)
68+
69+
for _, arg := range os.Args[1:] {
70+
f, err := os.Open(arg)
71+
if err != nil {
72+
log.Fatal(err)
73+
}
74+
defer f.Close()
75+
d := xml.NewDecoder(f)
76+
77+
for {
78+
t, err := d.Token()
79+
if err != nil {
80+
log.Fatal(err)
81+
}
82+
if t == nil {
83+
log.Fatalf("input file %s does not appear to be a JUnit XML file", arg)
84+
}
85+
// Inspect the top level DOM element and perform the appropriate action
86+
switch se := t.(type) {
87+
case xml.StartElement:
88+
switch se.Name.Local {
89+
case "testsuites":
90+
input := &api.TestSuites{}
91+
if err := d.DecodeElement(input, &se); err != nil {
92+
log.Fatal(err)
93+
}
94+
for _, suite := range input.Suites {
95+
suites.Merge("", suite)
96+
}
97+
case "testsuite":
98+
input := &api.TestSuite{}
99+
if err := d.DecodeElement(input, &se); err != nil {
100+
log.Fatal(err)
101+
}
102+
suites.Merge("", input)
103+
default:
104+
log.Fatal(fmt.Errorf("unexpected top level element in %s: %s", arg, se.Name.Local))
105+
}
106+
default:
107+
continue
108+
}
109+
break
110+
}
111+
}
112+
113+
var suiteNames []string
114+
for k := range suites {
115+
suiteNames = append(suiteNames, k)
116+
}
117+
sort.Sort(sort.StringSlice(suiteNames))
118+
output := &api.TestSuites{}
119+
120+
for _, name := range suiteNames {
121+
suite := suites[name]
122+
123+
out := &api.TestSuite{
124+
Name: name,
125+
NumTests: uint(len(suite.runs)),
126+
}
127+
128+
var keys []string
129+
for k := range suite.runs {
130+
keys = append(keys, k)
131+
}
132+
sort.Sort(sort.StringSlice(keys))
133+
134+
for _, k := range keys {
135+
testCase := suite.runs[k]
136+
out.TestCases = append(out.TestCases, testCase)
137+
switch {
138+
case testCase.SkipMessage != nil:
139+
out.NumSkipped++
140+
case testCase.FailureOutput != nil:
141+
out.NumFailed++
142+
}
143+
out.Duration += testCase.Duration
144+
}
145+
output.Suites = append(output.Suites, out)
146+
}
147+
148+
e := xml.NewEncoder(os.Stdout)
149+
e.Indent("", "\t")
150+
if err := e.Encode(output); err != nil {
151+
log.Fatal(err)
152+
}
153+
e.Flush()
154+
fmt.Fprintln(os.Stdout)
155+
}

tools/junitreport/pkg/api/types.go

+10-1
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,9 @@ type TestCase struct {
6060
// Name is the name of the test case
6161
Name string `xml:"name,attr"`
6262

63+
// Classname is an attribute set by the package type and is required
64+
Classname string `xml:"classname,attr,omitempty"`
65+
6366
// Duration is the time taken in seconds to run the test
6467
Duration float64 `xml:"time,attr"`
6568

@@ -68,14 +71,20 @@ type TestCase struct {
6871

6972
// FailureOutput holds the output from a failing test
7073
FailureOutput *FailureOutput `xml:"failure"`
74+
75+
// SystemOut is output written to stdout during the execution of this test case
76+
SystemOut string `xml:"system-out,omitempty"`
77+
78+
// SystemErr is output written to stderr during the execution of this test case
79+
SystemErr string `xml:"system-err,omitempty"`
7180
}
7281

7382
// SkipMessage holds a message explaining why a test was skipped
7483
type SkipMessage struct {
7584
XMLName xml.Name `xml:"skipped"`
7685

7786
// Message explains why the test was skipped
78-
Message string `xml:"message,attr"`
87+
Message string `xml:"message,attr,omitempty"`
7988
}
8089

8190
// FailureOutput holds the output from a failing test

0 commit comments

Comments
 (0)