-
Notifications
You must be signed in to change notification settings - Fork 896
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] TestAllJobs - create separate jobs for Auth and Firestore #7353
Conversation
|
Size Report 1Affected ProductsNo changes between base commit (e522056) and merge commit (eb95336).Test Logs |
Size Analysis Report 1Affected ProductsNo changes between base commit (e522056) and merge commit (eb95336).Test Logs |
package.json
Outdated
@@ -24,14 +24,18 @@ | |||
"dev": "lerna run --parallel --scope @firebase/* --scope firebase dev", | |||
"build": "lerna run --scope @firebase/* --scope firebase build", | |||
"build:changed": "ts-node-script scripts/ci-test/build_changed.ts", | |||
"build:ci:auth": "lerna run build:deps --scope '@firebase/auth*'", |
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.
Same comment regarding putting the command directly in the workflow.
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.
Done.
node scripts/print_test_logs.js | ||
env: | ||
FIREBASE_TOKEN: ${{ secrets.FIREBASE_CLI_TOKEN }} | ||
- name: Generate coverage file |
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.
I'm curious what the coverage step is going to do when it only has a subset of packages to work with but as it apparently currently doesn't work at all https://coveralls.io/github/firebase/firebase-js-sdk?branch=master I don't think it's an immediate concern. Future todo to figure out why it hasn't been working I guess.
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.
Yeah, I wasn't quite sure about this either.
Discussion
Created separate jobs in the Test All workflow for both Auth and Firestore. These two tests have a history of flaking when execute in the same job as the other tests. Breaking them-out into their own jobs may reduce the flake and isolate re-attempts at executing the test.
Also created a step to build the SDK. The various test domains (Auth, Firestore and the rest) no use the results from the build step to run the tests. This reduces the time needed to re-run flakey tests from 40 minutes to:
Testing
Updated the CI Workflow. Test has executed successfully (here's an example).
API Changes
N/A, CI and the addition of new build targets only.