Drop --release from but-sdk build to speed up CI#13699
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
mtsgrd
force-pushed
the
buildkite-test-results
branch
from
c680a72 to
cfc8e94
Compare
mtsgrd
force-pushed
the
buildkite-test-results
branch
from
cfc8e94 to
8543911
Compare
mtsgrd
requested review from
Byron,
Caleb-T-Owens,
estib-vega,
krlvi,
schacon and
slarse
as code owners
There was a problem hiding this comment.
Pull request overview
Wires multiple CI test suites in the monorepo (Playwright e2e, WebdriverIO blackbox, Rust workspace tests, and Desktop Vitest) to report results into Buildkite Test Engine for flaky-test detection and historical trend tracking.
Changes:
- Adds Buildkite Test Engine reporters for Playwright and Vitest and ensures
BUILDKITE_ANALYTICS_TOKENis available to Turbo tasks. - Adds JUnit output for WebdriverIO blackbox tests and uploads JUnit XML to Buildkite via
curl. - Switches Rust CI tests from
cargo testtocargo nextestwith a CI profile and uploads nextest JUnit output to Buildkite.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
turbo.json |
Passes Buildkite analytics token through Turbo for test tasks. |
pnpm-lock.yaml |
Locks new dependencies for Buildkite collector + WDIO JUnit reporter and their transitive deps. |
e2e/playwright/playwright.config.ts |
Adds Buildkite test collector Playwright reporter on CI. |
e2e/package.json |
Adds buildkite-test-collector and WDIO JUnit reporter to the e2e workspace package. |
e2e/blackbox/wdio.blackbox.conf.ts |
Configures WDIO to emit JUnit XML to test-results/results.xml. |
apps/desktop/vite.config.ts |
Enables Buildkite test collector Vitest reporter on CI. |
apps/desktop/package.json |
Adds buildkite-test-collector to desktop devDependencies. |
.github/workflows/test-e2e.yml |
Uploads blackbox JUnit XML to Buildkite; sets Playwright suite token env var. |
.github/workflows/push.yaml |
Runs Rust tests via nextest; uploads nextest JUnit XML; sets Vitest suite token env var. |
.config/nextest.toml |
Introduces a CI nextest profile and configures JUnit output path. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
|
@krlvi With the exception of the comment about doctests not being executed which I don't know off the top of my head, all of Copilot's comments are valid and should be addressed. Especially using a curl-installer in CI is bad for more reasons than Copilot points out (non-determinism is not one of them, though, odd choice of words). But I don't have anything worthwhile to add on top of what Copilot already pointed out. |
|
btw i should have added "for tomorrow" in my message 🙈 |
mtsgrd
force-pushed
the
buildkite-test-results
branch
from
c2594bb to
03bc689
Compare
mtsgrd
force-pushed
the
buildkite-test-results
branch
from
03bc689 to
a2a98e8
Compare
mtsgrd
force-pushed
the
buildkite-test-results
branch
from
a2a98e8 to
7ebde87
Compare
mtsgrd
force-pushed
the
buildkite-test-results
branch
from
7ebde87 to
4227b9d
Compare
Wire up four test suites to report results to Buildkite Test Engine for flaky test detection and trend tracking: - Playwright e2e: native buildkite-test-collector reporter - Blackbox (WebdriverIO): JUnit XML via @wdio/junit-reporter + curl upload - Cargo workspace: switch to cargo-nextest with JUnit output + curl upload - Desktop vitest: native buildkite-test-collector reporter Each suite uses its own BUILDKITE_SUITE_TOKEN_* GitHub Actions secret. Allow CI test uploads to continue on failure Make the CI investigation robust to partial failures by allowing the upload steps for test results to continue on error. This ensures that downstream test-reporting invocations don't block the workflow when previous steps are cancelled or fail. Also fix the e2e test JUnit reporter path to use an explicit path join so test artifacts are written to the intended test-results directory.
mtsgrd
force-pushed
the
buildkite-test-results
branch
from
4227b9d to
96151cb
Compare
|
@slarse we wouldn't need the curl installer if we ran all CI on our own image. Is that something you'd be into? *edit: I think we can also just install it the normal way if we whitelist something. |
mtsgrd
changed the title
@mtsgrd I seem to have missed this comment. Anyway, this isn't really a "curl installer" in the traditional sense, it's just fetching an archive using curl. Which isn't inherently bad, the badness about what I commented on was that it was a) fetching latest, and b) not validating what was fetched. a) seems to have been fixed but b) is still an issue, we should at the very least have checksum validation here. I'll add a PR for that. |
The
but-sdk-build-checkCI job compiles the SDK in release mode, which takes ~12 minutes. Since this job only verifies the build succeeds and types are up-to-date, debug mode is sufficient. Release artifacts go throughnapi prepublishseparately, so this doesn't affect actual releases.