-
Notifications
You must be signed in to change notification settings - Fork 532
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ci: Move full compat matrix tests for local-server and tinylicious to the e2e tests pipeline #20057
Conversation
tools/pipelines/build-client.yml
Outdated
- ci:test:realsvc:local | ||
- ci:test:realsvc:tinylicious | ||
- ${{ if ne(variables['System.TeamProject'], 'public') }}: | ||
- ${{ else }}: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only reason these were added because we shipped some serious bugs. I don't remember how bad they were, but it was a bad customer experience. It specifically tests loader and driver compat. I would recommend you move these tests to the e2e test pipelines as they aren't run there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That seems reasonable, I'll look into that. My take is that we can probably remove them here if we move them, since we do monitor the e2e tests pipeline closely to keep tinylicious test pass rate at 100%; we would still get notified even if a single test started failing over there.
"ci:test:realsvc:local:full": "pnpm run -r --no-sort --stream --no-bail test:realsvc:local:report:full", | ||
"ci:test:realsvc:local:full:coverage": "c8 --no-clean pnpm recursive --no-sort --stream --no-bail run test:realsvc:local:report:full", | ||
"ci:test:realsvc:tinylicious": "pnpm run -r --no-sort --stream --no-bail test:realsvc:tinylicious:report", | ||
"ci:test:realsvc:tinylicious:coverage": "c8 --no-clean pnpm run -r --no-sort --stream --no-bail test:realsvc:tinylicious:report ", | ||
"ci:test:realsvc:tinylicious:full": "pnpm run -r --no-sort --stream --no-bail test:realsvc:tinylicious:report:full", | ||
"ci:test:realsvc:tinylicious:full:coverage": "c8 --no-clean pnpm run -r --no-sort --stream --no-bail test:realsvc:tinylicious:report:full", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These four scripts became obsolete because the build-client and test-stability pipelines now only run their corresponding non-:full
versions. The :full
scripts now only exist in packages/test/test-end-to-end-tests/package.json
since that package itself is the only one that needs to run them (when it runs in the e2e tests pipeline).
Description
This moves the execution of full-compat-matrix e2e tests from the build-client pipeline to the e2e tests pipeline. This saves time on the "first step" of our CI workflows, the actual build (spends ~3m/9m running tests for local-server/tinylicious instead of ~12m/50m).
The extra time is now spent in the e2e tests pipeline (e.g. before ~3m/12m for local-server/tinylicious, and after, ~6m/40m).
The times above reflect specifically the time for the pipeline step that runs the tests, not the whole stage.
Note about previous version of PR
An initial version of this PR exposed a flag in the ADO UI so we could do "non-full-compat-matrix" testing for build - client in the internal project, to match what we do in the public project. That's still an option if we don't like moving the full-compat-matrix tests to the e2e tests pipeline only.
Description of that version
This exposes a new checkbox when manually running the `Build - client packages` pipeline in the internal ADO project, to allow us to kick off runs that don't run local-server/tinylicious tests with the **full** compat matrix (still uses some level of compat, but I'm not familiar enough to understand which kind exactly; it's the same one used by PR builds):So we can have runs that take this much time to run tests:
Instead of this much:
This is particularly useful when running
Build - client packages
not for the sake of generating artifacts to be released publicly, but for testing changes to the pipeline itself, or just need artifacts in order to test changes to a subsequent pipeline (e.g. one of our several test pipelines that require the published packages fromBuild - client packages
).Sample run (msft internal).
Reviewer Guidance
The review process is outlined on this wiki page.