Skip to content

dev/ci: integrate test reports with AnnotatedCmd

Warren Gifford requested to merge actually-add-test-reports into main

Created by: bobheadxi

Closes https://github.com/sourcegraph/sourcegraph/issues/31409 by integrating test reports scraping into AnnotatedCmd, removing a bunch of duplication and complexity from our test scripts and packaging test uploads into a neat pull-based API that allows local inspection of artefacts similarly to the annotations API.

Example:

JEST_JUNIT_OUTPUT_NAME="jest-junit.xml"
export JEST_JUNIT_OUTPUT_NAME
JEST_JUNIT_OUTPUT_DIR="$root_dir/test-reports"
export JEST_JUNIT_OUTPUT_DIR
mkdir -p "$JEST_JUNIT_OUTPUT_DIR"
yarn -s run test --maxWorkers 4 --verbose --testResultsProcessor jest-junit
	pipeline.AddStep(":jest::globe_with_meridians: Test",
		withYarnCache(),
		bk.AnnotatedCmd("dev/ci/yarn-test.sh client/web", bk.AnnotatedCmdOpts{
			TestReports: &bk.TestReportOpts{
				TestSuiteKeyVariableName: "BUILDKITE_ANALYTICS_FRONTEND_UNIT_TEST_SUITE_API_KEY",
			},
		}),

Stacked on https://github.com/sourcegraph/sourcegraph/pull/31965

API naming

I can't think of a better name than AnnotatedCmd at the moment. In a way, test analytics are "annotating" the build with extra test info, so it makes some sense, if potentially confusing. I'd like to keep everything together so exporters of test results can also create annotations.

If we do come up with a better name I'd vote to do that in a follow-up, since that would require a lot of renames and docs updates

Alternatives include:

  • ScrapedCmd(string, ScrapeOpts) (doesn't sound very elegant)
  • CmdWithExporters(string, ExportOpts) (wordy)
  • ExportedCmd(string, ExportOpts) (meaning a bit unclear)
  • Cmd(string, ...ExportOpts) (maybe? but somewhat of an abuse of varargs, or requires a bit of an API redesign)

Test plan

Ensure analytics still upload. See this build: https://buildkite.com/sourcegraph/sourcegraph/builds/134307

image image

Buildkite analytics reports attached to 4deec66026:

The above should be all the tests there are

Merge request reports

Loading