Switch to Github Actions #3152
Switch to Github Actions #3152
Conversation
|
Well that's good, CI triggered but it's obviously not ready yet. Still WIP. |
|
Well, tests work, lint doesn't yet, integration tests also don't work yet, build fails on windows, lint with reviewdog doesn't work on mac (do we care though? can we just run lint on one platform? Is that enough @mholt?) Gonna put this off until another day 😥 |
|
Why do we need to use "reviewdog" anyway - what is it? /cc @sarge about the integration tests |
|
Yes @mholt we can drop the integration tests, we can run them in process - so we don't have to start and stop a separate caddy process. |
golangci-lint wasn't installing on windows, so I googled alternatives and found that - thought it looked cool and figured it might be worth trying, adds comments inline to PRs for the reported issues. See https://github.com/reviewdog/action-golangci-lint I'm not married to it, I was just trying something 😛 Acknowledged @sarge I'll take that out 👍 |
francislavoie
force-pushed
the
v2-github-actions-test
branch
from
5fb61d2
to
99a4d82
Compare
|
I... think I did it, guys! 🎉 I'm not sure what to do about test/coverage reporting, would appreciate some thoughts there. Please review! |
francislavoie
changed the title
francislavoie
added
under review 🧐
There was a problem hiding this comment.
Thanks for tackling this, Francis! 😃 I'm catching up on the whole Github Actions stuff, but left a few comments for the stuff I know from the top of my head.
Now that we're using Github-native CI, can we add automated releases? I've seen a few actions that prepare releases as draft on pushes of tags. We just need to figure out how to stitch them together.
.github/workflows/ci.yml
Outdated
| - name: Checkout code | ||
| uses: actions/checkout@v2 | ||
|
|
||
| - name: Install test and coverage analysis tools |
There was a problem hiding this comment.
Those were needed because Azure uses JUnit XML format for test result reporting. I haven't found anything as straightforward for Github Actions. There's a workaround described here:
https://github.community/t5/GitHub-Actions/Publishing-Test-Results/m-p/37236/highlight/true#M2860
Seems like a bit of a hassle though. Let's research more. Otherwise, we'll have to pull more tooling, unfortunately.
.github/workflows/ci.yml
Outdated
| # TODO publish coverage/lint/test results ? | ||
|
|
||
| # To return the correct result even though we set 'continue-on-error: true' | ||
| - name: Coerce correct build result (Windows) |
There was a problem hiding this comment.
These were needed earlier to bypass the test failures so we can publish the test results. Keeping or removing this step depends on how end up setting up the publishing of test/coverage/lint results.
There was a problem hiding this comment.
Yep. I just wanted to keep these ideas intact, I wasn't sure how we'd do result reporting.
|
Fuzzing failure now is just due to the missing API key. @mholt I think we'll need you to set the secret. See the instructions here https://help.github.com/en/actions/configuring-and-managing-workflows/creating-and-storing-encrypted-secrets The name for the secret we need is |
|
Wow, great work @francislavoie! I can even see the test output in the Checks tab. I set the API key as a secret. Please take another look! This will be a relief to have working again :) |
francislavoie
force-pushed
the
v2-github-actions-test
branch
from
9878efb
to
90b3891
Compare
Is that different from a "crasher" caught by fuzzing? |
|
Yeah, regression testing fails for some reason. I think we might need to bug the fuzzit guys for some help. I have no idea what to do with that. @mohammed90 agreed we should just ignore it for now |
|
I opened an issue upstream. fuzzitdev/fuzzit#57 |
There was a problem hiding this comment.
This is awesome @francislavoie , thanks for the hard work!
If it's not too much trouble, could we add comments everywhere there is a commented section (or line) so that we can clearly remember/know why it is commented?
This may or may not be nicer than Azure Pipelines (same backend, different UI, slightly different config)
Supersedes #3149, trying to get CI to trigger.