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

Integration tests failing silently #737

Closed
mfisher87 opened this issue Jun 30, 2024 · 6 comments · Fixed by #738
Closed

Integration tests failing silently #737

mfisher87 opened this issue Jun 30, 2024 · 6 comments · Fixed by #738
Labels
automation CI, CD, or other automation bug Something isn't working

Comments

@mfisher87
Copy link
Collaborator

mfisher87 commented Jun 30, 2024

😱

https://github.com/nsidc/earthaccess/actions/runs/9666562376/job/26666365020

I believe part of the problem is poetry run, as I cannot reproduce this without Poetry.

@mfisher87 mfisher87 added bug Something isn't working automation CI, CD, or other automation labels Jun 30, 2024
@mfisher87
Copy link
Collaborator Author

I can't reproduce it with Poetry on my local machine either. But I can reproduce with Act:

| =========================== short test summary info ============================
| ERROR tests/integration/test_cloud_download.py - AssertionError: False is not true
| ERROR tests/integration/test_cloud_open.py - AssertionError: False is not true
| ERROR tests/integration/test_onprem_download.py - AssertionError: False is not true
| ERROR tests/integration/test_onprem_open.py - AssertionError: False is not true
| !!!!!!!!!!!!!!!!!!! Interrupted: 4 errors during collection !!!!!!!!!!!!!!!!!!!!
| ========================= 1 skipped, 4 errors in 2.08s =========================
[Integration Tests/integration-tests-3]   ✅  Success - Main Test

@mfisher87
Copy link
Collaborator Author

I can make the step fail by replacing poetry run ... with exit 1. Certainly would have been weird if not.

I can't get the step to fail by manually activating the venv with poetry out of the picture: . .venv/bin/activate && ./scripts/integration-test.sh. I'm very confused.

@mfisher87
Copy link
Collaborator Author

mfisher87 commented Jun 30, 2024

Just grasping at straws at this point. Entirely removing poetry from that workflow and replacing with pip, while it makes the workflow much simpler, does not cause the job to fail when expected.

Running pytest directly in the workflow without the script does not cause the job to fail when expected.

@mfisher87
Copy link
Collaborator Author

I think this is the oldest run on main which fails silently: https://github.com/nsidc/earthaccess/actions/runs/9035463040/job/24830297618

@mfisher87
Copy link
Collaborator Author

Finally! Got it to fail loudly by passing a path argument to pytest, e.g. pytest tests/integration instead of just pytest.

@mfisher87
Copy link
Collaborator Author

mfisher87 commented Jul 3, 2024

@betolink reminded me that we have code that allows up to 10% failure rate:

https://github.com/nsidc/earthaccess/blob/e7cb5bc948a88217b521ef76f7c93c9c384e9e3f/tests/integration/conftest.py

Right now, can print/log a message stating why the failure was tolerated so nobody goes off on a wild goose chase trying to figure this out 😆

Long term, the root problem is that we're testing against a random list of datasets. We should test against a fixed list of datasets that we expect to work (#215).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automation CI, CD, or other automation bug Something isn't working
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

1 participant