search

github / docs

The open-source repo for docs.github.com
https://docs.github.com
Creative Commons Attribution 4.0 International
16.15k stars 59.37k forks source link

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

Closed zeke closed 3 years ago

zeke commented 3 years ago

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

KieranHolroyd commented 3 years ago

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

chiedo commented 3 years ago

That's a good idea @KieranHolroyd! I'll explore some options on this.

chiedo commented 3 years ago

I think this is solved by this! https://github.com/github/docs/commit/c76bf47ea566f47e62e50abe454e078acb8c7a3c. Cool if I close @zeke?