X Tutup
The Wayback Machine - https://web.archive.org/web/20201012070857/https://github.com/github/docs/issues/350
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

run some tests conditionally to reduce overall PR check completion time #350

Open
zeke opened this issue Oct 9, 2020 · 1 comment
Open

run some tests conditionally to reduce overall PR check completion time #350

zeke opened this issue Oct 9, 2020 · 1 comment

Comments

@zeke
Copy link
Member

@zeke zeke commented Oct 9, 2020

We currently run all tests on every pull request. This is a good practice because it gives us confidence that our changes won't break anything. The downside of this setup is that even the smallest of pull requests have to run the whole test suite, including changes that don't affect the codebase, like the README or the contributing docs.

Some of our tests are still pretty slow:

Screen Shot 2020-10-09 at 8 29 21 AM

[ci skip] won't work

It's common for some projects to support a pattern like [ci skip] that can be added to commit messages to exempt the commit from triggering a new test run, but there are a few problems with that:

  1. It's kind of a dangerous pattern that can be abused by someone wishing to hastily jump the queue.
  2. We require PR branches to be up to date with the base branch before merging, so we almost always have to "update branch" before merging. This wouldn't skip CI.
  3. Our branch protection requires that certain checks pass. If those checks are skipped, the PR will never turn green.

what about detecting changed files?

There are lots of GitHub Actions out there for collecting a list of changed files in a pull request. This looks like a particularly nice one: https://github.com/futuratrepadeira/changed-files#readme

Using such an Action we could conditionally skip certain steps in our test suite. For example, we could skip the content test group if there have not been any changes to content/ or data/. We could also probably skip the links-and-images test in this case too.

The drawback of this approach is that we wouldn't have 100% confidence that every single PR is passing every single test, but I think that might be worth losing in exchange for a faster release cycle. 🚀

cc @github/docs-engineering

@zeke zeke added the engineering label Oct 9, 2020
@github-actions github-actions bot added the triage label Oct 9, 2020
@github-actions github-actions bot added this to Triage in Docs team reviews Oct 9, 2020
@janiceilene janiceilene moved this from Triage to Engineering in Docs team reviews Oct 9, 2020
@janiceilene janiceilene removed the triage label Oct 9, 2020
@KieranHolroyd
Copy link

@KieranHolroyd KieranHolroyd commented Oct 10, 2020

hypothetically, you could write a really simple cli to handle the conditional test suites, have that cli run the tests and aggregate the results somehow then publish them. allowing conditionality & an almost guarantee of the commit passing all the tests if the conditions are well defined

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.
X Tutup