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: only run tests when source changes #162

Closed
wants to merge 1 commit into from
Closed

ci: only run tests when source changes #162

wants to merge 1 commit into from

Conversation

ErikSchierboom
Copy link
Member

@ErikSchierboom ErikSchierboom commented Jan 29, 2021

To prevent the tests workflow from running unnecessarily (which is quite slow due to the slowness of the mac build), we'll only run it when either:

  1. Source code changes: *.nim, *.nims, or *.nimble files
  2. GitHub workflows change: .github/bin/* or .github/workflows/* files

I've verified this on my fork: https://github.com/ErikSchierboom/canonical-data-syncer/actions

Closes #141

@ErikSchierboom ErikSchierboom marked this pull request as draft January 29, 2021 11:44
@ErikSchierboom ErikSchierboom marked this pull request as ready for review January 29, 2021 11:48
on:
push:
paths:
- "**.nim"
Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

@ee7 ee7 left a comment

Choose a reason for hiding this comment

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

"**.cfg" could be another candidate for inclusion too, but we don't have one - it's fine.

I have some reservations that this could become outdated easily: I can imagine myself adding a new file, or moving some around, and not noticing that the test check is skipped (especially if there are many PRs, or more jobs/workflows in the future).

Maybe it would be safer to have the reverse filter? That is, an explicit list of files to skip.

I think the list of files that tests.yml effectively ignores with this PR is:

.github/CODEOWNERS
.github/dependabot.yml
.gitignore
LICENSE
README.md
configlet.version
scripts/fetch-configlet
scripts/fetch-configlet.ps1

That list is a bit longer than the list in the PR currently, and it might grow more quickly. But the failure mode seems better: a build when you didn't want one, rather than a PR passing when it should be failing.

What do you think?

To prevent the tests workflow from running unnecessarily we don't run it when Markdown files are changed
@ErikSchierboom
Copy link
Member Author

Yeah, there is a trade-off. Let's just simplify things and not run the tests when just Markdown files have changed. I have to look into how to encode that in the workflow.

@ErikSchierboom
Copy link
Member Author

I'm closing this in favor of #170 due to me having deleted my fork, which I apparently submitted the PR from.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Only run the tests when a source file has changed
2 participants