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

CI: Seperate out tests that fail due to HTTP calls #2012

Open
KennethEnevoldsen opened this issue Feb 7, 2025 · 4 comments · May be fixed by #2029
Open

CI: Seperate out tests that fail due to HTTP calls #2012

KennethEnevoldsen opened this issue Feb 7, 2025 · 4 comments · May be fixed by #2029

Comments

@KennethEnevoldsen
Copy link
Contributor

KennethEnevoldsen commented Feb 7, 2025

Currently, the test suite sometimes fails due to an error outside the scope of mteb (e.g. on HF). This makes it hard to trust when the test fails. A solution to this would be to simply factor out those tests in a separate CI (such that if the main tests fail it is clear easy to trust the error)

@Samoed and @isaac-chung WDYT about this?

thread on how to do this in pytest for reference

@Samoed
Copy link
Collaborator

Samoed commented Feb 7, 2025

Yes, we can separete integration tests

@sam-hey
Copy link
Contributor

sam-hey commented Feb 7, 2025

This might also be a solution: https://github.com/pytest-dev/pytest-rerunfailures

One argument in favor of this fix is that cases not covered otherwise might be missed and it is less work.

pytest --reruns 5 --only-rerun AssertionError --only-rerun ValueError --reruns-delay 1

@KennethEnevoldsen
Copy link
Contributor Author

pytest --reruns 5 --only-rerun AssertionError --only-rerun ValueError --reruns-delay 1

Hmm, but this could also hide an error that happens by chance.

@sam-hey
Copy link
Contributor

sam-hey commented Feb 7, 2025

I don't think that AssertionError and ValueError are merely placeholders for a ConnectionError or any other temporary issue. Since this error should disappear after a short time, rerunning the test with --reruns-delay xxxx should resolve the issue. However, if the problem persists, it might indicate a deeper issue that should be addressed by a developer as it will fail also after the reruns.

@sam-hey sam-hey linked a pull request Feb 11, 2025 that will close this issue
4 tasks
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 a pull request may close this issue.

3 participants