-
Notifications
You must be signed in to change notification settings - Fork 2
ci: Only run-tests if tests/demo/examples/core is modified #210
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
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #210 +/- ##
==========================================
- Coverage 76.11% 69.72% -6.40%
==========================================
Files 28 28
Lines 3082 3082
Branches 480 480
==========================================
- Hits 2346 2149 -197
- Misses 523 728 +205
+ Partials 213 205 -8 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Love this Angela! Will use something similar in one of my repos! What about |
Also, should probably consider adding |
Hmm isn't this pretty much all paths now? 😅 I suggest a slightly different approach:
- "tesseract_core/**"
- "tests/endtoend_tests/**"
- "demo/**"
- "examples/**"
- "production.uv.lock" (accepting the rare false-negative but that should be OK and immediately caught by the push to main test) This makes it so we don't need to run the expensive E2E tests if changes are isolated to |
I guess the concern I thought Jonathan might have (hence why I added) is that if the packages change, the tests may fail due to changes in the packages? But I supposed that would be relatively rare and could be caught in the regular benchmarking? I think currently, docs changes + workflow files other than e2e test file changes are excluded, but nothing else. Added base tests to exclusion list is a good catch as well. I don't quite understand the production.uv.lock actually -- i thought that's just generated based on the pyproject.toml / requirements? So would adding that be effectively the same as having pyproject.toml in the list? |
Yes, that's what I meant with my comment regarding false-negatives. It's not inconceivable that only an E2E test breaks due to a package update, but unlikely (and it would be caught immediately after pushing to
The lockfile contains specific versions, not abstract dependencies. |
@dionhaefner I did leave run_tests.yml in there since the tests should be run if the test configs are modified? |
Need to make some changes for end2end vs base* |
I saw that there's some hacky way about it using if conditions in the jobs and matching paths? but I think it's best to just split it into 2 workflow files for e2e vs base... |
Current version only runs tests-base but not e2e.. weird bc I added the .yml to the paths file so it should run, but good to see the base only part is working.. |
https://github.com/pasteurlabs/tesseract-core/actions/runs/15568981560/job/43840017745?pr=219 Did a test branch here and checked that e2e tests are not fully running when only doc is changed vs in current branch it is fullyg running e2e 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.
Pre-approved, this looks really clean now! Let's do the final cosmetics then merge.
Thanks @angela-ko
Need to do some changes to make it skip entire jobs (if possible) instead of steps -- but will do after dinner.. |
https://github.com/pasteurlabs/tesseract-core/actions/runs/15573704318/job/43854596186?pr=219 Ok reran the tests and now it's fully showing up as skipped (did not know if conditions could be put on the full job) ![]() vs tests are running here. Also looks a LOT cleaner now! |
Relevant issue or PR
#205
Description of changes
Add filter to only
run-tests
iftests
demo
examples
tesseract-core
is modifiedTesting done
This will be the test (if run-tests doesn't trigger)