refactor: Run unit tests separately from e2e tests #867
Conversation
faddat
requested review from
jtremback,
mpoke,
sainoe,
shaspitz,
MSalopek,
mmulji-ic,
glnro and
yaruwangway
as code owners
|
Cool. So is this going to show compatibility of builds on different OSs? |
|
yep :) - also osx-windows-linux |
|
Does this have any downsides? Does it slow down testing at all? Is it running the tests a bunch of different times? |
|
no. It is a standard github action with the matrix setup. It will always be dranatically faster than the job called "automated testing" it runs:
|
|
I'd like to get this merged, so that these tests are run separately from the automated tests. Right now the "automated tests" fail if the unit tests fail, and we don't get the feedback from the integration tests. |
faddat
changed the title
This comment was marked as off-topic.
This comment was marked as off-topic.
faddat
changed the title
| - name: E2E tests | ||
| run: make test-e2e-short | ||
| run: go run ./tests/e2e/... --happy-path-only |
There was a problem hiding this comment.
| run: go run ./tests/e2e/... --happy-path-only | |
| run: make test-e2e-short |
There was a problem hiding this comment.
oh -- actually I'd meant to add --verbose
There was a problem hiding this comment.
Please use make test-e2e-short. Developers can always run the tests locally with --verbose.
There was a problem hiding this comment.
But we can't prove outcomes using CI, so that there's no debate, in the way that you recommend.
CI is a neutral and clearly defined build environment that can be used as proof of issues.
There was a problem hiding this comment.
We want proof, there is no reason whatsoever not to capture the logs.
Co-authored-by: Marius Poke <marius.poke@posteo.de>
We have a limited number of workers, Marko was complaining about this earlier, so we need to rationalise how many tests we run & how often. Ditto with Gaia, coz they're already quite long, adding matrix testing will eat into our budget even more. Just FYI, we need to find a sweet spot - perhaps an exercise for later. |
|
@mmulji-ic so I've got to agree with you on this point. And the options are short of rough. I had thought about moving over to a gitlab that we sell host but this has other significant disadvantages. I'm curious to learn about what we spend on GitHub, and how to reduce that spend. At notional, we use only GitHub free features with the exception of copilot. When we accidentally enabled some kind of a corporate premium mode, we got a bill for 13500, I nearly had a heart attack. |
|
@faddat id recommend implementing the approach in the sdk here. This way we can be consistent with all repos. Secondly im not sure the need to test on different OS a single os is fine as we target linux machine for production instead of things like windows and mac. I think after adopting the sdk approach this would be good to go 😄 |
|
this log helped me to make a major step forward with 47ification: https://github.com/cosmos/interchain-security/actions/runs/4903809348/jobs/8756471785?pr=869 the way things are displayed matters, and helps human time is much more expensive than machine time |
|
I just want to say that I am comfortable asserting that this is the correct way for CI to work in this repository. You can close the pull request if you'd like to, but I believe that this is good. |
|
The idea here is solid but we're going to take a different direction |
Description
Closes #908
The venerable @mvdan has contributed many things to the golang ecosystem and it seems he has also made a nice matrix test setup. Having this in CI makes ensuring compatibility easier and makes test results visible.
This is a CI only change.
Alongside the initial changes, I've decoupled the workflow for unit tests and e2e tests so that failures in unit tests do not block e2e tests. This will make sure that failures are more visible in ci, making developers lives easier.
Type of change
If you've checked more than one of the first three boxes, consider splitting this PR into multiple PRs!
Feature: Changes and/or adds code behavior, irrelevant to bug fixesFix: Changes and/or adds code behavior, specifically to fix a bugRefactor: Changes existing code style, naming, structure, etc.Testing: Adds testingDocs: Adds documentationRegression tests
If
Refactor, describe the new or existing tests that verify no behavior was changed or added where refactors were introduced.New behavior tests
If
FeatureorFix, describe the new or existing tests that verify the new behavior is correct and expected.Versioning Implications
If the above box is checked, which version should be bumped?
MAJOR: Consensus breaking changes to both the provider and consumers(s), including updates/breaking changes to IBC communication between provider and consumer(s)MINOR: Consensus breaking changes which affect either only the provider or only the consumer(s)PATCH: Non consensus breaking changesTargeting
Please select one of the following: