Closed alimi closed 4 years ago
This is great! Love having tests run for every push and PR. Kind of concerning that one check passes (for the push) while the other fails (for the PR), though ... Both of the rspec logs show an ERROR
around line 54, but one test run counts that as a pass, while the other counts it as a fail. It would be nice to figure out why, if there's something in the environment that needs to be set or changed?
Yeah, there are a few flaky tests floating around. Lemme see if I can figure out what's going on with the one that failed on the PR build.
This is awesome, even the flaky tests, which it's nice to get some visibility on. We might want to turn the volume down on the spec output, I think we could set the log_level in config.yml.test to be error
rather than warn
to shut up all those log lines.
Couple of questions:
1) why are there two test runs, one for PR and one for Push? 2) Does the status badge reflect the most recent push to master or does it just reflect whatever branch was run most recently?
1. why are there two test runs, one for PR and one for Push?
Yep, one was triggered by opening the PR and the other was triggered by pushing my last commit.
It doesn't show up in the PR checks, but there are also builds for every commit I pushed up. If you look through my commits, you'll see a green check for commits that had green builds and red x's for commits that had bad builds.
2. Does the status badge reflect the most recent push to master or does it just reflect whatever branch was run most recently?
I thought it was based of the last push to master, but it looks like it's using the last build. I think it makes more sense to base it off of the last push to master. Lemme see if we can change it.
I've made a few changes. First, the flaky test has been fixed in https://github.com/mountetna/magma/pull/151/commits/fd9d8c3cb60c901503118f69578cc690cacadb33.
https://github.com/mountetna/magma/pull/151/commits/21eb90d052481f72e23e5fdbbea1a5da5b1576d6 changes the log levels so we won't see so much noise in the test run output.
Finally per the conversation we had in Slack yesterday, we'll run the test suite twice to try to catch any flaky tests that creep into the test suite. We'll only do the second run if the first run passes because if the first run fails we'll already know if there's something we need to fix. So far the double run hasn't caught any flaky tests 🤞🏾.
Also, there is a small difference between the two runs that get triggered by a PR and push. The push run is based off of the last pushed commit, while the PR run is based on a potential merge commit of the PR into master. So the PR run gives us a little more confidence that the changes will work with whatever is on master. h/t @notmarkmiranda
The status badge is based off of master by default so nothing we need to change there. https://help.github.com/en/actions/configuring-and-managing-workflows/configuring-a-workflow#adding-a-workflow-status-badge-to-your-repository
:ship:
This PR sets up a GitHub Actions workflow that will run the test suite on every push and PR.
Here are a couple of example runs:
If you click on the "build" link on the left sidebar, you can see all the steps that happened for each run.
GitHub Actions also gives us a status badge that I've included in the README.
Magma changes
There are a few changes we had to make so GitHub Actions could run the test suite.
Gemfile
andGemfile.lock
. Bundler uses theGemfile.lock
version (5.28.0).config.yml.test
. The workflow will copy that file toconfig.yml
so it can boot magma.