-
Notifications
You must be signed in to change notification settings - Fork 846
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
Allow xdc and ndc tests in single test CI mode #6636
base: main
Are you sure you want to change the base?
Conversation
.github/workflows/run-tests.yml
Outdated
description: "Run a single unit test (INSTEAD of functional test)" | ||
type: boolean | ||
default: false | ||
run_single_test_mode: |
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.
can this be a single 'enum' like choice?
like 'test_mode' with values
'all -fund-tests'
'single-func-test'
'single-unit-test'
etc.
I guess this will greatly simplify the logic below
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 tried to make an "all-tests" option that is the default test_mode
and is used when the action is run for a PR, but the default behavior when the action is run for a PR is to make all the workflow_dispatch
inputs empty, so inputs.test_mode
was blank instead of the default all_tests
option.
There may be a way to set it though.. I agree it would make the code much less annoying
…sts successful in single test mode
db36d20
to
3d2ddfa
Compare
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.
@carlydf, would you fix the merge conflicts?
@@ -1,5 +1,7 @@ | |||
name: All Tests | |||
|
|||
run-name: ${{ inputs.test_name }}${{ inputs.test_dbs }}${{ inputs.run_single_test_job }} |
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.
Does this need any delimiters to make it readable? How does it affect tests that aren't run in "single" mode?
|
||
- run: make build-tests | ||
if: ${{ inputs.run_single_test_job == '' }} |
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.
Why is this change required?
name: Unit test | ||
needs: [pre-build, set-up-single-test] | ||
strategy: | ||
fail-fast: false | ||
runs-on: ubuntu-20.04 | ||
env: | ||
BUILDKITE_MESSAGE: '{"job": "unit-test"}' |
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.
Thanks!
SINGLE_TEST_ARGS: ${{ needs.set-up-single-test.outputs.single_test_args }} | ||
UNIT_TEST_DIR: ${{ inputs.unit_test_directory }} | ||
TEST_TIMEOUT: ${{ needs.set-up-single-test.outputs.test_timeout }} |
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 aren't relevant anymore, they moved into the Run unit tests
step. PTAL.
env: | ||
PERSISTENCE_TYPE: ${{ matrix.persistence_type }} | ||
PERSISTENCE_DRIVER: ${{ matrix.persistence_driver }} | ||
BUILDKITE_MESSAGE: '{"job": "functional-test-xdc", "db": "${{ matrix.persistence_driver }}"}' | ||
TEST_TIMEOUT: ${{ needs.set-up-single-test.outputs.test_timeout }} |
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: Set this in Run functional test xdc
so it's clear that it doesn't apply to the flaky test detection step.
What changed?
Why?
How did you test it?
Potential risks
Documentation
Is hotfix candidate?