-
Notifications
You must be signed in to change notification settings - Fork 77
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 path filter #857
Add path filter #857
Conversation
Codecov Report
@@ Coverage Diff @@
## add-tests-to-tips-example #857 +/- ##
==========================================================
Coverage 64.61% 64.61%
==========================================================
Files 107 107
Lines 9281 9281
==========================================================
Hits 5997 5997
Misses 3284 3284 Continue to review full report at Codecov.
|
static_react_task: | ||
- 'examples/static_react_task/**' | ||
static_react_task_with_tips: | ||
- 'examples/static_react_task_with_tips/**' | ||
mnist: | ||
- 'examples/remote_procedure/mnist/**' | ||
template: | ||
- 'examples/remote_procedure/template/**' | ||
toxicity_detection: | ||
- 'examples/remote_procedure/toxicity_detection/**' | ||
abstractions: | ||
- 'mephisto/abstractions/**' | ||
data_model: | ||
- 'mephisto/data_model/**' | ||
operations: | ||
- 'mephisto/operations/**' | ||
tools: | ||
- 'mephisto/tools/**' | ||
mephisto-task: | ||
- 'packages/mephisto-task/src/**' | ||
mephisto-worker-addons: | ||
- 'packages/mephisto-worker-addons/src/**' |
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 are the folders that I thought can potentially break a task, so that is why they trigger the tasks
static-react-task: | ||
needs: changes | ||
if: ${{ (needs.changes.outputs.static_react_task == 'true') || (needs.changes.outputs.mephisto-task == 'true') || (needs.changes.outputs.abstractions == 'true') || (needs.changes.outputs.data_model == 'true') || (needs.changes.outputs.operations == 'true') || (needs.changes.outputs.tools == '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.
If any of the folders in this statement are modified (== 'true'), then run the test. Otherwise, skip it
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.
This is done for every job in the action. Some of the predicates in the if are changed depending of the job.
For example, static_react_task_with_tips is also looking in the mephisto-worker-addons directory as it has Tips and Feedback in its core_components.jsx.
- uses: actions/checkout@v3 | ||
with: | ||
fetch-depth: 2 | ||
- uses: dorny/paths-filter@v2 |
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.
This GitHub action is used to allow for path filtering on the job level.
GitHub actions natively supports path filtering on the action level. This is too broad for this use-case as it would look something like examples/**.
This would mean that all the tests would run every time a file is changed in static_react_task.
This solution allows only for the static_react_task test to run when a file is changed in static_react_task
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.
This is awesome. Filtering will definitely help overall!
One small change I'd make though, we'd expect that changes to the underlying architecture that could break mnist
and toxicity_detection
(our slowest tests here) would also break template
, so we can drop the
(needs.changes.outputs.abstractions == 'true') || (needs.changes.outputs.data_model == 'true') || (needs.changes.outputs.operations == 'true') || (needs.changes.outputs.tools == 'true')
triggers from these cases. Otherwise this is a solid DevX improvement!
Overview
The base branch is looked at(in this case
add-tests-to-tips-example
) to determine what files are changed.As commit 62faad8 updates a file in the static_react_task folder, the static_react_task test gets ran and the others don't.