-
Notifications
You must be signed in to change notification settings - Fork 12
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
Add test seed upload workflow. #1117
Conversation
8f885a5
to
6b81f9d
Compare
6b81f9d
to
4916c97
Compare
✅ Test Seed Generated SuccessfullyTo test the new seed, launch the browser with the following command line:
Seed Details
|
99fe40e
to
78094f7
Compare
69bed89
to
86cf705
Compare
const comment = require('.github/workflows/scripts/comment.js') | ||
await comment(github, context, commentBody) | ||
|
||
- name: Install dependencies and run tests |
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.
nit: could we split it into?
- npm install
- build proto & typecheck
- tests
const comment = require('.github/workflows/scripts/comment.js') | ||
await comment(github, context, commentBody) | ||
|
||
- name: Comment "Upload Failed" |
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 would prefer to move this up and rename to "Upload skipped due to a build/lint/test failure".
Not sure that we should make a fallback for s3 uploading / invalidation, because it's a very small chance of failure here. IMHO, if it happens it's enough to have "Upload In Progress" comment and the failed job in the list.
REMOTE_SEED_PATH: 'pull/${{ github.event.pull_request.number }}/seed' | ||
|
||
steps: | ||
- uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4 |
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.
nit: do we need this?
cebc893
to
e961aaf
Compare
jobs: | ||
build: | ||
runs-on: ubuntu-latest | ||
if: github.event.pull_request.merged == true |
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.
what if the PR is declined?
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.
ah, we don't need this check at all.
e961aaf
to
8334f2e
Compare
25e40aa
to
cfe4139
Compare
Add workflows for upload and removal of test seed.
Add a single study to test these workflows.