Skip to content
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

keep going when single test runs fail, but exit make signaling error #204

Merged

Conversation

hotzevzl
Copy link
Member

@hotzevzl hotzevzl commented May 24, 2021

So far we've been ignoring non-zero exit codes from the two test runs for each of unit and e2e test recipes in GH Action, leading to tests being shown as always passing even if one or both of the test runs failed.

This PR adds --keep-going to avoid stopping the recipe, while propagating error conditions to the make exit code.

How to test

Good luck 😬 - I suggest to test locally by breaking one of the e2e tests for the API service on purpose (as these run before the geoprocessing e2e tests), and verifying that the geoprocessing tests still run, and that final teardown happens, but make exits with a non-zero exit code.

@vercel
Copy link

vercel bot commented May 24, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployments, click below or on the icon next to each commit.

marxan – ./app

🔍 Inspect: https://vercel.com/vizzuality1/marxan/78manfQh5jzXFJtE1YmzdJ4r95Gy
✅ Preview: https://marxan-git-chore-apimarxan-434propagate-errors-t-394bb7.vercel.app

marxan-storybook – ./app

🔍 Inspect: https://vercel.com/vizzuality1/marxan-storybook/B6STWkuS9SSLQS4qfiv8dfTQbX6r
✅ Preview: https://marxan-storybo-git-chore-apimarxan-434propagate-errors-t-c77b66.vercel.app

@hotzevzl hotzevzl requested review from aagm and kgajowy May 24, 2021 08:27
@kgajowy
Copy link
Contributor

kgajowy commented May 24, 2021

@hotzevzl hope this works as the pipeline wasn't triggered in this PR

@hotzevzl
Copy link
Member Author

hotzevzl commented May 24, 2021

@hotzevzl hope this works as the pipeline wasn't triggered in this PR

yep, no pipeline runs were triggered as the change was in the root of the repo, while the e2e pipeline is triggered only when there are changes in the api or geoprocessing subtrees: I triggered a manual run to test (https://github.com/Vizzuality/marxan-cloud/runs/2644588837) but it looks like we've only had all-green e2e runs for a while, so we'd need to purposefully break at least a test to check that this works in GH Actions.

Locally this works for me if I break a test on purpose: here is the final excerpt of a local test run showing the API tests failing, the geoprocessing ones running nevertheless, and make eventually terminating after teardown with non-zero exit code:

marxan-test-unit-api | PASS src/modules/scenarios/wdpa-area-calculation.service.spec.ts
marxan-test-unit-api | PASS src/modules/scenarios/dto/update-scenario-planning-unit-lock-status-dto.spec.ts
marxan-test-unit-api | PASS src/modules/ping/ping.controller.spec.ts
marxan-test-unit-api | PASS src/modules/analysis/providers/planning-units/adapters/async-jobs-adapter.spec.ts
marxan-test-unit-api | PASS src/modules/analysis/providers/status/scenario-status.service.spec.ts
marxan-test-unit-api |
marxan-test-unit-api | Snapshot Summary
marxan-test-unit-api |  › 1 snapshot failed from 1 test suite. Inspect your code changes or run `yarn run test:unit:ci -u` to update them.
marxan-test-unit-api |
marxan-test-unit-api | Test Suites: 1 failed, 14 passed, 15 total
marxan-test-unit-api | Tests:       1 failed, 48 passed, 49 total
marxan-test-unit-api | Snapshots:   1 failed, 12 passed, 13 total
marxan-test-unit-api | Time:        16.367 s
marxan-test-unit-api | error Command failed with exit code 1.
marxan-test-unit-api | info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
marxan-test-unit-api exited with code 1
Aborting on container exit...
ERROR: 1
make[1]: *** [Makefile:146: test-unit-api] Error 1
# run unit tests - geoprocessing
docker-compose -f docker-compose-test-unit.yml up --abort-on-container-exit --exit-code-from geoprocessing geoprocessing
Building with native build. Learn about native build in Compose here: https://docs.docker.com/go/compose-native-build/
WARNING: Found orphan containers (marxan-test-e2e-redis, marxan-test-e2e-postgresql-api, marxan-test-e2e-postgresql-geo-api) for this project. If you removed or renamed this service in your compose file, you can run this command with the --remove-orphans flag to clean it up.
Starting marxan-test-unit-geoprocessing ... done
Attaching to marxan-test-unit-geoprocessing
marxan-test-unit-geoprocessing | Running Unit Tests
marxan-test-unit-geoprocessing | yarn run v1.22.5
marxan-test-unit-geoprocessing | $ jest --silent
marxan-test-unit-geoprocessing | PASS src/app.controller.spec.ts
marxan-test-unit-geoprocessing |
marxan-test-unit-geoprocessing | Test Suites: 1 passed, 1 total
marxan-test-unit-geoprocessing | Tests:       1 passed, 1 total
marxan-test-unit-geoprocessing | Snapshots:   0 total
marxan-test-unit-geoprocessing | Time:        1.446 s, estimated 7 s
marxan-test-unit-geoprocessing | Done in 3.83s.
marxan-test-unit-geoprocessing exited with code 0
Aborting on container exit...
make[1]: Target 'test-unit-backend' not remade because of errors.
make[1]: Leaving directory 'marxan-cloud'
make: *** [Makefile:155: run-test-unit] Error 2

@aagm aagm merged commit f784aaa into develop May 25, 2021
@kgajowy kgajowy deleted the chore/api/MARXAN-434_propagate-errors-through-ci-pipeline branch May 25, 2021 09:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants