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 doesn't always run the service tests #2314

Closed
paulmelnikow opened this issue Nov 14, 2018 · 5 comments
Closed

CI doesn't always run the service tests #2314

paulmelnikow opened this issue Nov 14, 2018 · 5 comments
Labels
bug Bugs in badges and the frontend developer-experience Dev tooling, test framework, and CI

Comments

@paulmelnikow
Copy link
Member

CircleCI builds often don't correctly detect that they are on a PR. Currently the only way to know whether or not the service tests ran is to inspect the build logs.

That's because there is no way to reliably distinguish branch and PR builds in CircleCI.

Somtimes the cause is having CircleCI set up on the fork where the PR originates, triggering a branch build ahead of time, though we've also had cases where this happened on forks without CircleCI. Plus Dependabot branches are never on a fork.

As it turns out, there is a workaround suggested by Danger: configure CircleCI to run only on PRs.

That would necessitate either:

  1. A separate CI service – or simply a separate repo – to run the daily tests
  2. A PR-triggered bot that triggers the service-test builds on Circle programmatically
  3. A cron-triggered bot that triggers the nightly Circle build programmatically

I experimented in #1936 with a radically different alternative, which is to specify the affected services in the commit history. This has an advantage of making development a bit easier, but it didn't really pan out. Maybe that work could be harvested for a dev-only tool.

@paulmelnikow paulmelnikow added bug Bugs in badges and the frontend developer-experience Dev tooling, test framework, and CI labels Nov 14, 2018
@chris48s
Copy link
Member

A separate CI service – or simply a separate repo – to run the daily tests

This seems like the quickest/simplest thing to do. Also refs #1584

@paulmelnikow
Copy link
Member Author

I went ahead and set up another repo with Circle, and got Coveralls working as well. (Thanks for the reminder about that!)

The reason I chose Circle is because #979 was a long-standing issue solved by Circle, and I didn't want to solve that problem again.

I turned on "Only build PRs" for the main repo. I think the service test runner should start working reliably now, without other changes. Though let's see!

This was referenced Nov 16, 2018
paulmelnikow added a commit that referenced this issue Nov 17, 2018
- Stop running daily service tests in the main repo (since they're now handled [over here](https://github.com/badges/daily-tests)
- Add coverage and separate daily tests badges with links to coveralls
- Update our coverage ignores
    - Move scripts, which do not need coverage, into `scripts/`
- Split out coverage test for npm package
- Remove spurious env var

Ref: #1584 #2314
@paulmelnikow
Copy link
Member Author

This is looking good. Coveralls is looking nice, too!

@chris48s
Copy link
Member

Just thinking about this is slightly more detail, was the CI_PULL_REQUEST variable the thing that we couldn't rely on?

If so, now that we only run on PR, should we also remove the checks on that e.g:

if [[ ! -z $CI_PULL_REQUEST ]]; then

@paulmelnikow
Copy link
Member Author

Yea, that was the main thing. Yep, for sure, those checks can go.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bugs in badges and the frontend developer-experience Dev tooling, test framework, and CI
Projects
None yet
Development

No branches or pull requests

2 participants