Repository navigation
Write a JUnit XML report from databricks pipelines test - #7299
tharunbhuk-db wants to merge 9 commits into
Conversation
cf934e0 to
cc2e61a
Compare
2c84257 to
8542365
Compare
|
Commit: Regression test reportSelected 9 changed tests (limit=20, 0 not selected) 9 tests: regression=9 unreleased=0 coverage=0 failing=0
TestAccept/pipelines/databricks-cli-help: branch ✅ | main (aed6702) ❌ | latest (v1.23.0) ❌main (aed6702): latest (v1.23.0): same output as main (aed6702) TestAccept/pipelines/test/arguments: branch ✅ | main (aed6702) ❌ | latest (v1.23.0) ❌main (aed6702): latest (v1.23.0): TestAccept/pipelines/test/immutable: branch ✅ | main (aed6702) ❌ | latest (v1.23.0) ❌main (aed6702): latest (v1.23.0): same output as main (aed6702) TestAccept/pipelines/test/many-cases: branch ✅ | main (aed6702) ❌ | latest (v1.23.0) ❌main (aed6702): latest (v1.23.0): same output as main (aed6702) TestAccept/pipelines/test/pipeline-id: branch ✅ | main (aed6702) ❌ | latest (v1.23.0) ❌main (aed6702): latest (v1.23.0): same output as main (aed6702) TestAccept/pipelines/test/results: branch ✅ | main (aed6702) ❌ | latest (v1.23.0) ❌main (aed6702): latest (v1.23.0): same output as main (aed6702) TestAccept/pipelines/test/run-control: branch ✅ | main (aed6702) ❌ | latest (v1.23.0) ❌main (aed6702): latest (v1.23.0): same output as main (aed6702) 2 more failure details omitted, see the full report in the run. |
Integration test reportCommit: 7c61f68
Top 20 slowest tests (at least 2 minutes):
|
cb0a526 to
025649f
Compare
cc2e61a to
b4df318
Compare
…pdate that is not a test update
…pace.sync_mode immutable
Co-authored-by: Isaac <no-reply@databricks.com>
b4df318 to
7c61f68
Compare
025649f to
23e0bb0
Compare
| } | ||
| data = append([]byte(xml.Header), data...) | ||
| data = append(data, '\n') | ||
| if err := os.WriteFile(path, data, 0o644); err != nil { |
There was a problem hiding this comment.
os.WriteFile truncates first, so a crash mid-write leaves a half-written file that CI reads as broken. Could we use atomicfile.Write from libs/atomicfile? bundle/deploy/state_pull.go already uses it.
| // A parameter ID in brackets, such as test_cast[a::int], stays with the test name. | ||
| func junitCaseName(testID string) (string, string) { | ||
| params := "" | ||
| if i := strings.Index(testID, "["); i >= 0 { |
There was a problem hiding this comment.
strings.Index(testID, "[") searches the whole ID, so tests/[weird]/test_a.py::test_x gets classname "" and name tests.[weird].test_a.py::test_x. Searching only after the first :: fixes it. Worth adding that case to TestJUnitCaseName.
|
|
||
| if problem != "" { | ||
| suite.Cases = append(suite.Cases, junitTestCase{ | ||
| Name: "update " + updateID, |
There was a problem hiding this comment.
This entry is named update <id>, while pre-update errors are named pipelines test (line 167). On the timeout path, the update is still running and the message doesn't start with RUN INCOMPLETE:. Could we use one naming scheme, with a clear message for the local-timeout case?
| // Every exit writes the report, so that an older report never survives. | ||
| report := &testReport{path: junitPath, out: cmd.OutOrStdout(), suiteName: pipelineID} | ||
| if report.suiteName == "" && len(args) > 0 { | ||
| report.suiteName = args[0] |
There was a problem hiding this comment.
Outside a project, positional args are selections, so pipelines test tests/test_x.py --junit-xml r.xml names the suite tests/test_x.py, and with no args the suite name is empty. Could we use the pipeline name or ID once it resolves?
|
|
||
| --junit-xml writes one entry per test with its outcome and duration. An | ||
| incomplete run, a run without tests, or a timeout adds a failing entry that | ||
| names the cause. With workspace.sync_mode immutable, test names keep the path |
There was a problem hiding this comment.
Non-blocking: thanks for documenting this. Classnames that change with each deploy break CI test-history views (flaky detection, trends). A follow-up could name cases relative to the snapshot root.
| trace musterr $CLI pipelines test --pipeline-id 1234 --no-wait --junit-xml out.no-wait.xml | ||
|
|
||
| title "An invalid flag replaces an older report" | ||
| echo '<testsuites><testsuite name="1234" tests="1"><testcase name="test_old"></testcase></testsuite></testsuites>' > out.flag-error.xml |
There was a problem hiding this comment.
Non-blocking: the "an older report is replaced" check now appears in five scripts: arguments, submit-error, run-control, outside, and an invalid project. Each shows the same thing, that a failure before the update still rewrites the report. One case before the request and one at submit would cover it, and the other goldens could go.
🥞 Stacked PR
Changes
--junit-xml <path>, which writes onetestcaseper collected test case, with its outcome and duration. Cases are named relative to the project folder, as pytest names them, so the names do not depend on the working directory. Withworkspace.sync_mode: immutable, the names keep the deployed snapshot path, which changes with each deployment; the help says so.error. A run without tests, an incomplete run, a timeout or a late update failure adds a failing entry that names the cause.--resumeof an update that is not a test update, so an older report is never left behind.--junit-xmlcannot be used with--no-wait, and the--resumecommand printed after a timeout keeps--junit-xmlso that it replaces the partial report.Why
CI systems read JUnit XML to show test results and to gate merges.
Tests
out.*.xml) for a passing run, each failing outcome, invalid flags, a timeout and resume, and errors before the update is read.This pull request and its description were written by Isaac.