diff --git a/pkg/jobrunaggregator/jobrunaggregatoranalyzer/analyzer_test.go b/pkg/jobrunaggregator/jobrunaggregatoranalyzer/analyzer_test.go index b818ef9f0bb..ad5ecbd7ec2 100644 --- a/pkg/jobrunaggregator/jobrunaggregatoranalyzer/analyzer_test.go +++ b/pkg/jobrunaggregator/jobrunaggregatoranalyzer/analyzer_test.go @@ -480,6 +480,40 @@ func TestAggregateTestCasePropagatesLifecycle(t *testing.T) { assert.Equal(t, "informing", combined.Lifecycle) } +func TestAggregateTestCasePropagatesProperties(t *testing.T) { + combined := &junit.TestCase{Name: "test-with-properties"} + + source := &junit.TestCase{ + Name: "test-with-properties", + Properties: []*junit.Property{ + {Name: "lifecycle", Value: "informing"}, + {Name: "owner", Value: "team-platform"}, + }, + } + + err := aggregateTestCase("suite", combined, "logs/job", "run1", source) + assert.NoError(t, err) + assert.Equal(t, 2, len(combined.Properties)) + assert.Equal(t, "lifecycle", combined.Properties[0].Name) + assert.Equal(t, "informing", combined.Properties[0].Value) + assert.Equal(t, "owner", combined.Properties[1].Name) + assert.Equal(t, "team-platform", combined.Properties[1].Value) + + // Second aggregation should not overwrite the properties + source2 := &junit.TestCase{ + Name: "test-with-properties", + Properties: []*junit.Property{ + {Name: "lifecycle", Value: "informing"}, + }, + } + err = aggregateTestCase("suite", combined, "logs/job", "run2", source2) + assert.NoError(t, err) + // Properties should remain unchanged from first aggregation + assert.Equal(t, 2, len(combined.Properties)) + assert.Equal(t, "lifecycle", combined.Properties[0].Name) + assert.Equal(t, "informing", combined.Properties[0].Value) +} + func TestInformingTestFailureMessage(t *testing.T) { suite := &junit.TestSuites{ Suites: []*junit.TestSuite{ diff --git a/pkg/jobrunaggregator/jobrunaggregatoranalyzer/junit.go b/pkg/jobrunaggregator/jobrunaggregatoranalyzer/junit.go index 57c705f4433..94292fa2eb9 100644 --- a/pkg/jobrunaggregator/jobrunaggregatoranalyzer/junit.go +++ b/pkg/jobrunaggregator/jobrunaggregatoranalyzer/junit.go @@ -235,5 +235,16 @@ func aggregateTestCase(testSuiteName string, combined *junit.TestCase, jobGCSBuc combined.Lifecycle = toAdd.Lifecycle } + // Propagate properties from source test case to the combined one + if len(combined.Properties) == 0 && len(toAdd.Properties) > 0 { + combined.Properties = make([]*junit.Property, len(toAdd.Properties)) + for i, prop := range toAdd.Properties { + combined.Properties[i] = &junit.Property{ + Name: prop.Name, + Value: prop.Value, + } + } + } + return nil } diff --git a/pkg/junit/censor.go b/pkg/junit/censor.go index 320aa0c3c5e..878fc3376b7 100644 --- a/pkg/junit/censor.go +++ b/pkg/junit/censor.go @@ -7,30 +7,35 @@ func CensorTestSuite(censor secretutil.Censorer, testSuite *TestSuite) { if testSuite == nil { return } - testSuite.Name = censored(censor, testSuite.Name) - for i := range testSuite.Properties { - testSuite.Properties[i].Name = censored(censor, testSuite.Properties[i].Name) - testSuite.Properties[i].Value = censored(censor, testSuite.Properties[i].Value) + censorStr(censor, &testSuite.Name) + for i, prop := range testSuite.Properties { + censorStr(censor, &prop.Name, &prop.Value) + testSuite.Properties[i] = prop } - for i := range testSuite.TestCases { - testSuite.TestCases[i].Name = censored(censor, testSuite.TestCases[i].Name) - if testSuite.TestCases[i].SkipMessage != nil { - testSuite.TestCases[i].SkipMessage.Message = censored(censor, testSuite.TestCases[i].SkipMessage.Message) + for i, testCase := range testSuite.TestCases { + censorStr(censor, &testCase.Name) + for j, prop := range testCase.Properties { + censorStr(censor, &prop.Name, &prop.Value) + testCase.Properties[j] = prop } - if testSuite.TestCases[i].FailureOutput != nil { - testSuite.TestCases[i].FailureOutput.Output = censored(censor, testSuite.TestCases[i].FailureOutput.Output) - testSuite.TestCases[i].FailureOutput.Message = censored(censor, testSuite.TestCases[i].FailureOutput.Message) + if testCase.SkipMessage != nil { + censorStr(censor, &testCase.SkipMessage.Message) } - testSuite.TestCases[i].SystemOut = censored(censor, testSuite.TestCases[i].SystemOut) - testSuite.TestCases[i].SystemErr = censored(censor, testSuite.TestCases[i].SystemErr) + if testCase.FailureOutput != nil { + censorStr(censor, &testCase.FailureOutput.Output, &testCase.FailureOutput.Message) + } + censorStr(censor, &testCase.SystemOut, &testCase.SystemErr) + testSuite.TestCases[i] = testCase } for i := range testSuite.Children { CensorTestSuite(censor, testSuite.Children[i]) } } -func censored(censor secretutil.Censorer, value string) string { - raw := []byte(value) - censor.Censor(&raw) - return string(raw) +func censorStr(censor secretutil.Censorer, values ...*string) { + for _, val := range values { + raw := []byte(*val) + censor.Censor(&raw) + *val = string(raw) + } } diff --git a/pkg/junit/censor_test.go b/pkg/junit/censor_test.go index 9b8e635e1d9..201003eb469 100644 --- a/pkg/junit/censor_test.go +++ b/pkg/junit/censor_test.go @@ -13,13 +13,16 @@ func TestCensorTestSuite(t *testing.T) { censorer.Refresh("secret") input := TestSuite{ Name: "some secret", - Properties: []*TestSuiteProperty{ + Properties: []*Property{ {Name: "secret things", Value: "secret values"}, {Name: "also secret things", Value: "really secret values"}, }, TestCases: []*TestCase{ { - Name: "somehow secret", + Name: "somehow secret", + Properties: []*Property{ + {Name: "lifecycle", Value: "secret value"}, + }, SkipMessage: &SkipMessage{Message: "skipped due to secret"}, FailureOutput: &FailureOutput{ Message: "failed due to secret", @@ -29,7 +32,10 @@ func TestCensorTestSuite(t *testing.T) { SystemErr: "error containing secret", }, { - Name: "somehow also secret", + Name: "somehow also secret", + Properties: []*Property{ + {Name: "owner", Value: "also secret value"}, + }, SkipMessage: &SkipMessage{Message: "also skipped due to secret"}, FailureOutput: &FailureOutput{ Message: "also failed due to secret", @@ -42,13 +48,16 @@ func TestCensorTestSuite(t *testing.T) { Children: []*TestSuite{ { Name: "some nested secret", - Properties: []*TestSuiteProperty{ + Properties: []*Property{ {Name: "nested secret things", Value: "nested secret values"}, {Name: "also nested secret things", Value: "really nested secret values"}, }, TestCases: []*TestCase{ { - Name: "somehow nested secret", + Name: "somehow nested secret", + Properties: []*Property{ + {Name: "lifecycle", Value: "nested secret value"}, + }, SkipMessage: &SkipMessage{Message: "skipped due to nested secret"}, FailureOutput: &FailureOutput{ Message: "failed due to nested secret", @@ -58,7 +67,10 @@ func TestCensorTestSuite(t *testing.T) { SystemErr: "error containing nested secret", }, { - Name: "somehow also nested secret", + Name: "somehow also nested secret", + Properties: []*Property{ + {Name: "owner", Value: "also nested secret value"}, + }, SkipMessage: &SkipMessage{Message: "also skipped due to nested secret"}, FailureOutput: &FailureOutput{ Message: "also failed due to nested secret", @@ -71,13 +83,16 @@ func TestCensorTestSuite(t *testing.T) { Children: []*TestSuite{ { Name: "some very nested secret", - Properties: []*TestSuiteProperty{ + Properties: []*Property{ {Name: "very nested secret things", Value: "very nested secret values"}, {Name: "also very nested secret things", Value: "really very nested secret values"}, }, TestCases: []*TestCase{ { - Name: "somehow very nested secret", + Name: "somehow very nested secret", + Properties: []*Property{ + {Name: "lifecycle", Value: "very nested secret value"}, + }, SkipMessage: &SkipMessage{Message: "skipped due to very nested secret"}, FailureOutput: &FailureOutput{ Message: "failed due to very nested secret", @@ -87,7 +102,10 @@ func TestCensorTestSuite(t *testing.T) { SystemErr: "error containing very nested secret", }, { - Name: "somehow also very nested secret", + Name: "somehow also very nested secret", + Properties: []*Property{ + {Name: "owner", Value: "also very nested secret value"}, + }, SkipMessage: &SkipMessage{Message: "also skipped due to very nested secret"}, FailureOutput: &FailureOutput{ Message: "also failed due to very nested secret", diff --git a/pkg/junit/testdata/zz_fixture_TestCensorTestSuite.yaml b/pkg/junit/testdata/zz_fixture_TestCensorTestSuite.yaml index 15a56c22bed..513fea815bc 100644 --- a/pkg/junit/testdata/zz_fixture_TestCensorTestSuite.yaml +++ b/pkg/junit/testdata/zz_fixture_TestCensorTestSuite.yaml @@ -28,6 +28,12 @@ Children: Space: "" Lifecycle: "" Name: somehow very nested XXXXXX + Properties: + - Name: lifecycle + Value: very nested XXXXXX value + XMLName: + Local: "" + Space: "" SkipMessage: Message: skipped due to very nested XXXXXX XMLName: @@ -48,6 +54,12 @@ Children: Space: "" Lifecycle: "" Name: somehow also very nested XXXXXX + Properties: + - Name: owner + Value: also very nested XXXXXX value + XMLName: + Local: "" + Space: "" SkipMessage: Message: also skipped due to very nested XXXXXX XMLName: @@ -88,6 +100,12 @@ Children: Space: "" Lifecycle: "" Name: somehow nested XXXXXX + Properties: + - Name: lifecycle + Value: nested XXXXXX value + XMLName: + Local: "" + Space: "" SkipMessage: Message: skipped due to nested XXXXXX XMLName: @@ -108,6 +126,12 @@ Children: Space: "" Lifecycle: "" Name: somehow also nested XXXXXX + Properties: + - Name: owner + Value: also nested XXXXXX value + XMLName: + Local: "" + Space: "" SkipMessage: Message: also skipped due to nested XXXXXX XMLName: @@ -148,6 +172,12 @@ TestCases: Space: "" Lifecycle: "" Name: somehow XXXXXX + Properties: + - Name: lifecycle + Value: XXXXXX value + XMLName: + Local: "" + Space: "" SkipMessage: Message: skipped due to XXXXXX XMLName: @@ -168,6 +198,12 @@ TestCases: Space: "" Lifecycle: "" Name: somehow also XXXXXX + Properties: + - Name: owner + Value: also XXXXXX value + XMLName: + Local: "" + Space: "" SkipMessage: Message: also skipped due to XXXXXX XMLName: diff --git a/pkg/junit/types.go b/pkg/junit/types.go index e36dec60226..999acad9515 100644 --- a/pkg/junit/types.go +++ b/pkg/junit/types.go @@ -38,7 +38,7 @@ type TestSuite struct { Duration float64 `xml:"time,attr"` // Properties holds other properties of the test suite as a mapping of name to value - Properties []*TestSuiteProperty `xml:"properties>property,omitempty"` + Properties []*Property `xml:"properties>property,omitempty"` // TestCases are the test cases contained in the test suite TestCases []*TestCase `xml:"testcase"` @@ -47,8 +47,8 @@ type TestSuite struct { Children []*TestSuite `xml:"testsuite"` } -// TestSuiteProperty contains a mapping of a property name to a value -type TestSuiteProperty struct { +// Property contains a mapping of a property name to a value +type Property struct { XMLName xml.Name `xml:"property"` Name string `xml:"name,attr"` @@ -71,6 +71,9 @@ type TestCase struct { // Lifecycle indicates the test lifecycle phase (e.g. "informing" or "blocking") Lifecycle string `xml:"lifecycle,attr,omitempty"` + // Properties holds other properties of the test case as a mapping of name to value + Properties []*Property `xml:"properties>property,omitempty"` + // SkipMessage holds the reason why the test was skipped SkipMessage *SkipMessage `xml:"skipped"` diff --git a/test/e2e/observer/artifacts/multi-observers-junit_operator.xml b/test/e2e/observer/artifacts/multi-observers-junit_operator.xml index df8ffd26b45..563f89ecf86 100644 --- a/test/e2e/observer/artifacts/multi-observers-junit_operator.xml +++ b/test/e2e/observer/artifacts/multi-observers-junit_operator.xml @@ -1,21 +1,35 @@ - - - + + + + + + + + + + + echo 'this is going to fail' this is going to fail + exit 1 {"component":"entrypoint","error":"wrapped process failed: exit status 1","file":"sigs.k8s.io/prow/pkg/entrypoint/run.go","func":"sigs.k8s.io/prow/pkg/entrypoint.Options.internalRun","level":"error","msg":"Error executing test process","severity":"error","time":"whatever"} error: failed to execute wrapped command: exit status 1 - - + + + + + + + The collected steps of multi-stage phase post. + The collected steps of multi-stage phase pre. + The collected steps of multi-stage phase test. diff --git a/test/e2e/observer/artifacts/simple-observer-junit_operator.xml b/test/e2e/observer/artifacts/simple-observer-junit_operator.xml index 44cafda2492..1db30d8d2fa 100644 --- a/test/e2e/observer/artifacts/simple-observer-junit_operator.xml +++ b/test/e2e/observer/artifacts/simple-observer-junit_operator.xml @@ -1,18 +1,29 @@ - + + + + The collected steps of multi-stage phase post. + The collected steps of multi-stage phase pre. + The collected steps of multi-stage phase test. - - - + + + + + + + + + \ No newline at end of file diff --git a/test/e2e/simple/artifacts/junit_operator.xml b/test/e2e/simple/artifacts/junit_operator.xml index 89637c78075..bc9f0984749 100644 --- a/test/e2e/simple/artifacts/junit_operator.xml +++ b/test/e2e/simple/artifacts/junit_operator.xml @@ -1,10 +1,20 @@ - - - - - + + + + + + + + + + + + + + + \ No newline at end of file