-
Notifications
You must be signed in to change notification settings - Fork 246
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
feat_: functional tests coverage #5805
Conversation
Jenkins BuildsClick to see older builds (172)
|
b2cd29f
to
383e145
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## develop #5805 +/- ##
==========================================
Coverage ? 11.56%
==========================================
Files ? 834
Lines ? 151028
Branches ? 0
==========================================
Hits ? 17473
Misses ? 131476
Partials ? 2079
Flags with carried forward coverage won't be shown. Click here to find out more. |
6e498f0
to
4ae7b1f
Compare
6e498f0
to
c24eba8
Compare
You'll have to fix what user the tests in the container run as:
Which means it should run as user with |
@jakubgs thanks you! Indeed, I completely forgot that all of this is executed in docker 🤦 I have 2 questions though.
|
|
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.
I see you changed user
in integration-tests/docker-compose.test.status-go.yml
, but what about integration-tests/docker-compose.anvil.yml
?
The coverage report is empty, because With a simple |
Alright, shutdown fixed by changing the stop signal:
Seems that |
But now there's a new issue. UPD: never mind, the testswere just never launched. Because one of the anvil containers didn't start. |
Now the tests are simply failing 🤔 Error not in chain call
error="anvil:8545.error: Post \"http://anvil:8545\": dial tcp: lookup anvil on 127.0.0.11:53: server misbehaving"
chain=31337 OK, this is fixed now. |
daad5db
to
c1eb094
Compare
11e500b
to
e745d4d
Compare
@status-im/devops can I please have your eyes on this as well? 🙏 |
e745d4d
to
c3ab708
Compare
@@ -494,18 +494,15 @@ test-verif-proxy-wrapper: | |||
CGO_CFLAGS="$(CGO_CFLAGS)" go test -v github.com/status-im/status-go/rpc -tags gowaku_skip_migrations,nimbus_light_client -run ^TestProxySuite$$ -testify.m TestRun -ldflags $(LDFLAGS) | |||
|
|||
|
|||
run-integration-tests: SHELL := /bin/sh |
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.
Why are we removing the SHELL
override here? The script just depends on Docker right? And that's always provided by the system, never by Nix shell, so this override is valid.
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.
@jakubgs this is because of codecov
CLI tool, which we have as nix derivation:
url = "https://cli.codecov.io/v${version}/${platform}/codecov"; |
It's the only thing, but I didn't see a simple way to avoid it here 🤷
@@ -42,11 +58,11 @@ pipeline { | |||
always { | |||
script { | |||
archiveArtifacts( | |||
artifacts: '**/results.xml', | |||
artifacts: 'integration-tests/reports/*.xml, integration-tests/*.log, integration-tests/coverage/coverage.html', |
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.
If this is a list we should indicate it as such:
artifacts: 'integration-tests/reports/*.xml, integration-tests/*.log, integration-tests/coverage/coverage.html', | |
artifacts: ['integration-tests/reports/*.xml, integration-tests/*.log, integration-tests/coverage/coverage.html'], |
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.
06edb43
to
28e19e4
Compare
Accompanying changes
Moved the whole
run-integration-tests
target to_assets/scripts/run_integration_tests.sh
Because it became too long to be in the
Makefile
.Fixed coverage uploading to codecov
There was a missing
upload-process
command.Refactored things a bit to use relatives path and not to have looong lines
Fixed
.codecov.yml
fileFixed integration tests run to properly shutdown statusd with
stop_signal: SIGINT
Added logs and coverage CI artifacts
Run statusd containers with
INTEGRATION_TESTS_DOCKER_UID
userFunctional tests coverage
Implemented according to https://go.dev/blog/integration-test-coverage
You can see the results in the Codecov comment (to be improved):
Or at the codecov report page: filtered for
functional
flag: