From 919c521ca0e111020773bce856e50b291bde54c4 Mon Sep 17 00:00:00 2001 From: Clayton Coleman Date: Wed, 28 Dec 2016 16:24:31 -0500 Subject: [PATCH] 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. --- test/extended/conformance.sh | 2 + test/extended/core.sh | 2 + test/extended/setup.sh | 15 ++- tools/junitmerge/junitmerge.go | 155 +++++++++++++++++++++++++++++ tools/junitreport/pkg/api/types.go | 11 +- 5 files changed, 183 insertions(+), 2 deletions(-) create mode 100644 tools/junitmerge/junitmerge.go diff --git a/test/extended/conformance.sh b/test/extended/conformance.sh index 2b745cf96848..8f36b5e8e265 100755 --- a/test/extended/conformance.sh +++ b/test/extended/conformance.sh @@ -29,4 +29,6 @@ TEST_PARALLEL="${PARALLEL_NODES:-5}" FOCUS="${pf}" SKIP="${ps}" TEST_REPORT_FILE os::log::info "Running serial tests" 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=$? +os::test::extended::merge_junit + exit $exitstatus diff --git a/test/extended/core.sh b/test/extended/core.sh index 8ffa1c360818..18926db15551 100755 --- a/test/extended/core.sh +++ b/test/extended/core.sh @@ -34,4 +34,6 @@ os::log::info "" os::log::info "Running serial tests" 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=$? +os::test::extended::merge_junit + exit $exitstatus diff --git a/test/extended/setup.sh b/test/extended/setup.sh index b48bc55ad028..1129277807c6 100644 --- a/test/extended/setup.sh +++ b/test/extended/setup.sh @@ -3,7 +3,7 @@ # This abstracts starting up an extended server. # If invoked with arguments, executes the test directly. -function os::test::extended::focus { +function os::test::extended::focus () { if [[ $# -ne 0 ]]; then os::log::info "Running custom: $*" os::test::extended::test_list "$@" @@ -27,6 +27,7 @@ function os::test::extended::setup () { os::util::ensure::built_binary_exists 'openshift' os::util::ensure::built_binary_exists 'oadm' os::util::ensure::built_binary_exists 'oc' + os::util::ensure::built_binary_exists 'junitmerge' 'tools/junitmerge' # ensure proper relative directories are set export EXTENDED_TEST_PATH="${OS_ROOT}/test/extended" @@ -244,6 +245,18 @@ function os::test::extended::test_list () { } readonly -f os::test::extended::test_list +# Merge all of the JUnit output files in the TEST_REPORT_DIR into a single file. +# This works around a gap in Jenkins JUnit reporter output that double counts skipped +# files until https://github.com/jenkinsci/junit-plugin/pull/54 is merged. +function os::test::extended::merge_junit () { + local output + output="$( mktemp )" + "$( os::util::find::built_binary junitmerge )" "${TEST_REPORT_DIR}"/*.xml > "${output}" + rm "${TEST_REPORT_DIR}"/*.xml + mv "${output}" "${TEST_REPORT_DIR}/junit.xml" +} +readonly -f os::test::extended::merge_junit + # Not run by any suite readonly EXCLUDED_TESTS=( "\[Skipped\]" diff --git a/tools/junitmerge/junitmerge.go b/tools/junitmerge/junitmerge.go new file mode 100644 index 000000000000..5fe862a5246f --- /dev/null +++ b/tools/junitmerge/junitmerge.go @@ -0,0 +1,155 @@ +package main + +import ( + "encoding/xml" + "log" + "os" + + "fmt" + "github.com/openshift/origin/tools/junitreport/pkg/api" + "sort" +) + +type uniqueSuites map[string]*suiteRuns + +func (s uniqueSuites) Merge(namePrefix string, suite *api.TestSuite) { + name := suite.Name + if len(namePrefix) > 0 { + name = namePrefix + "/" + } + existing, ok := s[name] + if !ok { + existing = newSuiteRuns(suite) + s[name] = existing + } + + existing.Merge(suite.TestCases) + + for _, suite := range suite.Children { + s.Merge(name, suite) + } +} + +type suiteRuns struct { + suite *api.TestSuite + runs map[string]*api.TestCase +} + +func newSuiteRuns(suite *api.TestSuite) *suiteRuns { + return &suiteRuns{ + suite: suite, + runs: make(map[string]*api.TestCase), + } +} + +func (r *suiteRuns) Merge(testCases []*api.TestCase) { + for _, testCase := range testCases { + existing, ok := r.runs[testCase.Name] + if !ok { + r.runs[testCase.Name] = testCase + continue + } + switch { + case testCase.SkipMessage != nil: + // if the new test is a skip, ignore it + case existing.SkipMessage != nil && testCase.SkipMessage == nil: + // always replace a skip with a non-skip + r.runs[testCase.Name] = testCase + case existing.FailureOutput == nil && testCase.FailureOutput != nil: + // replace a passing test with a failing test + r.runs[testCase.Name] = testCase + } + } +} + +func main() { + log.SetFlags(0) + suites := make(uniqueSuites) + + for _, arg := range os.Args[1:] { + f, err := os.Open(arg) + if err != nil { + log.Fatal(err) + } + defer f.Close() + d := xml.NewDecoder(f) + + for { + t, err := d.Token() + if err != nil { + log.Fatal(err) + } + if t == nil { + log.Fatalf("input file %s does not appear to be a JUnit XML file", arg) + } + // Inspect the top level DOM element and perform the appropriate action + switch se := t.(type) { + case xml.StartElement: + switch se.Name.Local { + case "testsuites": + input := &api.TestSuites{} + if err := d.DecodeElement(input, &se); err != nil { + log.Fatal(err) + } + for _, suite := range input.Suites { + suites.Merge("", suite) + } + case "testsuite": + input := &api.TestSuite{} + if err := d.DecodeElement(input, &se); err != nil { + log.Fatal(err) + } + suites.Merge("", input) + default: + log.Fatal(fmt.Errorf("unexpected top level element in %s: %s", arg, se.Name.Local)) + } + default: + continue + } + break + } + } + + var suiteNames []string + for k := range suites { + suiteNames = append(suiteNames, k) + } + sort.Sort(sort.StringSlice(suiteNames)) + output := &api.TestSuites{} + + for _, name := range suiteNames { + suite := suites[name] + + out := &api.TestSuite{ + Name: name, + NumTests: uint(len(suite.runs)), + } + + var keys []string + for k := range suite.runs { + keys = append(keys, k) + } + sort.Sort(sort.StringSlice(keys)) + + for _, k := range keys { + testCase := suite.runs[k] + out.TestCases = append(out.TestCases, testCase) + switch { + case testCase.SkipMessage != nil: + out.NumSkipped++ + case testCase.FailureOutput != nil: + out.NumFailed++ + } + out.Duration += testCase.Duration + } + output.Suites = append(output.Suites, out) + } + + e := xml.NewEncoder(os.Stdout) + e.Indent("", "\t") + if err := e.Encode(output); err != nil { + log.Fatal(err) + } + e.Flush() + fmt.Fprintln(os.Stdout) +} diff --git a/tools/junitreport/pkg/api/types.go b/tools/junitreport/pkg/api/types.go index 4092f530a76e..58339044c589 100644 --- a/tools/junitreport/pkg/api/types.go +++ b/tools/junitreport/pkg/api/types.go @@ -60,6 +60,9 @@ type TestCase struct { // Name is the name of the test case Name string `xml:"name,attr"` + // Classname is an attribute set by the package type and is required + Classname string `xml:"classname,attr,omitempty"` + // Duration is the time taken in seconds to run the test Duration float64 `xml:"time,attr"` @@ -68,6 +71,12 @@ type TestCase struct { // FailureOutput holds the output from a failing test FailureOutput *FailureOutput `xml:"failure"` + + // SystemOut is output written to stdout during the execution of this test case + SystemOut string `xml:"system-out,omitempty"` + + // SystemErr is output written to stderr during the execution of this test case + SystemErr string `xml:"system-err,omitempty"` } // SkipMessage holds a message explaining why a test was skipped @@ -75,7 +84,7 @@ type SkipMessage struct { XMLName xml.Name `xml:"skipped"` // Message explains why the test was skipped - Message string `xml:"message,attr"` + Message string `xml:"message,attr,omitempty"` } // FailureOutput holds the output from a failing test