-
Notifications
You must be signed in to change notification settings - Fork 4
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
Add unit test in Github action CI. #83
Conversation
2ec7891
to
f655684
Compare
53b5d2c
to
600329d
Compare
.github/workflows/on-pr.yml
Outdated
|
||
- name: Run unit tests | ||
run: | ||
PGPASSWORD=password make statedifftest |
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.
The password for test db is also in the Makefile. Why do we need it here?
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.
Added this into Makefile as env.
platform: [ubuntu-latest] | ||
runs-on: ${{ matrix.platform }} | ||
steps: | ||
- name: Create GOPATH |
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.
The first three steps here are the same as the earlier job. Can't we factor this out and run the tests in the same folder?
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.
Yes, We can but then the tests will run sequentially in one job.
Currently, we are running all the tests in parallel hence the setup steps need to be done for each individual test.
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.
Let me explore this a bit. I think we can parallelize the job after the initial setup.
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.
LGTM!
No description provided.