Skip to content
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: skip tests if unaffected TASK-1316 #5295

Merged
merged 28 commits into from
Dec 2, 2024
Merged

ci: skip tests if unaffected TASK-1316 #5295

merged 28 commits into from
Dec 2, 2024

Conversation

p2edwards
Copy link
Contributor

@p2edwards p2edwards commented Nov 23, 2024

👀 Preview steps

  1. 🟢 See the screenshots, and the "Checks" section of this PR
  2. 🟢 See a run summary

💭 Notes

  • A PR that only modifies a few Markdown files would look like this:
  • This PR (modifies the darker.yml, npm-test.yml, and pytest.yml workflow files, so all the jobs are run:)

@p2edwards p2edwards added the workflow Related to development process label Nov 28, 2024
@p2edwards p2edwards force-pushed the phil/ci-paths-filter branch from de99c3f to d3b9d92 Compare December 1, 2024 03:26
@p2edwards p2edwards changed the title ci: skip tests if unaffected (WIP) ci: skip tests if unaffected (WIP) [TASK-1316] Dec 1, 2024
@p2edwards p2edwards changed the title ci: skip tests if unaffected (WIP) [TASK-1316] ci: skip tests if unaffected (WIP) TASK-1316 Dec 1, 2024
@p2edwards p2edwards force-pushed the phil/ci-paths-filter branch from 6f64040 to d84067d Compare December 2, 2024 03:22
@p2edwards p2edwards force-pushed the phil/ci-paths-filter branch from d84067d to 7580008 Compare December 2, 2024 03:23
Looks like the checks at the bottom of the PR follows the order defined
in ci.yml.

Place npm-test after the python jobs since the npm-test is a 4-entry
matrix right now and buries the others below the scroll fold

Place pytest on top. it's usually there while you're waiting since it
takes the longest, so this time it doesn't have to move.

Make both files use the same order.

Give "detect changed files" a name to make it easier to find when
you're inspecting results
this code solves a rare problem and could go in a separate PR after
discussion

the "unknown files" detection would support this scenario:

- a new config file or folder gets merged in without being included
  in filters.yml.
- time passes.
- a small pr modifies that config, breaking the ci job, which is skipped
  because filters.yml is out of date.
@p2edwards p2edwards changed the title ci: skip tests if unaffected (WIP) TASK-1316 ci: skip tests if unaffected TASK-1316 Dec 2, 2024
@p2edwards p2edwards marked this pull request as ready for review December 2, 2024 08:45
@p2edwards p2edwards requested a review from Akuukis December 2, 2024 08:46
@Akuukis Akuukis force-pushed the phil/ci-paths-filter branch from 33ad903 to df60d52 Compare December 2, 2024 11:25
Copy link
Contributor

@Akuukis Akuukis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, works for me

  1. comment out CI lines
  2. make a bogus change within jsapp
  3. git push
  4. 🟢 notice selective CI runs
  5. git rest HEAD^
  6. git push -f

image

Comment on lines +6 to +15
darker:
- '{kpi,kobo,hub}/**/*.py' # .py
- 'pyproject.toml' # rules
- '.github/workflows/darker.yml' # ci

pytest:
- '{kpi,kobo,hub}/**/*.!(md)' # backend
- 'dependencies/**/*.!(md)' # pip
- 'pyproject.toml' # (can affect build/tests)
- '.github/workflows/pytest.yml' # ci
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

backend-related paths that will trigger darker/pytest

cc @noliveleger

Comment on lines +17 to +23
npm-test:
- '{jsapp,test,webpack,static,scripts}/**/*.!(md|py|sh|bash)' # frontend
- '{package*.json,patches/*.patch,scripts/copy_fonts.py}' # npm + postinstall
- '{tsconfig.json,.swcrc,.babelrc*,.browserslistrc}' # compilers
- '{.editorconfig,.prettier*,.stylelint*,.eslint*,coffeelint*}' # linters
- '.gitignore' # (can affect tools)
- '.github/workflows/npm-test.yml' # ci
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

frontend-related paths that will trigger npm-test

cc @magicznyleszek

@p2edwards p2edwards merged commit 0711df4 into main Dec 2, 2024
15 checks passed
@p2edwards p2edwards deleted the phil/ci-paths-filter branch December 2, 2024 17:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
workflow Related to development process
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants