-
Notifications
You must be signed in to change notification settings - Fork 75
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
Create GH Actions workflow for build&test of Go client #2318
Changes from all commits
8cc2e7e
ab6fdea
e59dfd9
5affb78
372333d
a368ae6
c41fb94
1947d44
0504b8c
8b9bd59
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
name: Go | ||
|
||
#TODO: extend the conditions once workflow gets tested together with other workflows | ||
on: | ||
push: | ||
branches: | ||
- master | ||
pull_request: | ||
workflow_dispatch: | ||
|
||
jobs: | ||
build-and-test: | ||
runs-on: ubuntu-latest | ||
steps: | ||
- uses: actions/checkout@v1 | ||
- uses: satackey/[email protected] | ||
continue-on-error: true # ignore the failure of a step and avoid terminating the job | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll flag that the comment here mostly seems to be repeating what the flag already communicates (unlike There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're right. I'm not changing it though, as this fragment of code will be removed once #2331 gets merged to master. |
||
- name: Run Docker build | ||
run: | | ||
docker build \ | ||
--target gobuild \ | ||
--tag go-build-env . | ||
docker build \ | ||
--tag keep-client . | ||
- name: Create test results directory | ||
run: | | ||
mkdir test-results | ||
- name: Run Go tests | ||
run: | | ||
docker run \ | ||
--volume $GITHUB_WORKSPACE/test-results:/mnt/test-results \ | ||
--workdir /go/src/github.com/keep-network/keep-core \ | ||
go-build-env \ | ||
gotestsum --junitfile /mnt/test-results/unit-tests.xml | ||
- name: Publish unit test results | ||
uses: EnricoMi/[email protected] | ||
if: always() # guarantees that this action always runs, even if earlier steps fail | ||
with: | ||
github_token: ${{ secrets.GITHUB_TOKEN }} | ||
files: ./test-results/unit-tests.xml | ||
check_name: Go Test Results # name under which test results will be presented in GitHub (optional) | ||
comment_on_pr: false # turns off commenting on Pull Requests | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If things are working correctly, I believe the check should already be bubbling the important parts of this information up, including line annotations, e.g. like https://github.com/keep-network/keep-ecdsa/actions/runs/555260607 . I don't think we need the junit intermediary here, see https://github.com/actions/toolkit/blob/master/docs/commands.md#problem-matchers and the setup-go action's problem matcher at https://github.com/actions/setup-go/blob/main/matchers.json . There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (Of note: we may want to add our own matcher that works with our go-to test output.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One thing that may be worth considering here, if we're continuing to run the Circle build, is to change this filter to run on certain branches (e.g.
rfc-18/*
) and change Circle to not run on that filter. This effectively gives you a branch namespace to use for the work.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implemented in #2329