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

Skip test case in pypy or windows #3986

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

ocelotl
Copy link
Contributor

@ocelotl ocelotl commented Jun 20, 2024

Fixes #3985

@ocelotl ocelotl self-assigned this Jun 20, 2024
@ocelotl ocelotl requested a review from a team as a code owner June 20, 2024 21:35
@ocelotl ocelotl changed the title Skip test case when running in Windows Skip test case when running in Windows or pypy Jun 20, 2024
@xrmx
Copy link
Contributor

xrmx commented Jun 20, 2024

I would bump the delta to 30 instead

@ocelotl
Copy link
Contributor Author

ocelotl commented Jun 27, 2024

I would bump the delta to 30 instead

Next time it can fail with a delta of 31 🤷 I don't see much sense in chasing this issue by increasing the delta, as far as we know this can fail with pretty much any delta but we are more certain that this happens only on Pypy or Windows.

@ocelotl ocelotl added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Jun 27, 2024
@xrmx
Copy link
Contributor

xrmx commented Jun 28, 2024

I would bump the delta to 30 instead

Next time it can fail with a delta of 31 🤷 I don't see much sense in chasing this issue by increasing the delta, as far as we know this can fail with pretty much any delta but we are more certain that this happens only on Pypy or Windows.

I think it would be nice to try to keep coverage on Windows I guess

@ocelotl
Copy link
Contributor Author

ocelotl commented Jun 28, 2024

I would bump the delta to 30 instead

Next time it can fail with a delta of 31 🤷 I don't see much sense in chasing this issue by increasing the delta, as far as we know this can fail with pretty much any delta but we are more certain that this happens only on Pypy or Windows.

I think it would be nice to try to keep coverage on Windows I guess

It would be great to have this test run on Windows, but it randomly fails there and that is out of our control. By the way, here here it failed with a delta of 34:

self.assertAlmostEqual((export_time - start_time) * 1e3, 500, delta=25)
E       AssertionError: 534.8496437072754 != 500 within 25 delta (34.84964370727539 difference)

So, either we skip this test case, or we automate what we always do, which is rerunning this test case everytime it fails.

@ocelotl ocelotl changed the title Skip test case when running in Windows or pypy Rerun test case when running in Windows or pypy Jul 3, 2024
@ocelotl ocelotl changed the title Rerun test case when running in Windows or pypy Rerun test case when it fails in Windows or pypy Jul 3, 2024
@ocelotl
Copy link
Contributor Author

ocelotl commented Jul 3, 2024

I modified this PR so that the test case will be rerun if it fails (3 times maximum) with an AssertionError and if being run in Windows or with PyPy.

@ocelotl ocelotl changed the title Rerun test case when it fails in Windows or pypy Rerun test case when it fails Jul 3, 2024
@ocelotl
Copy link
Contributor Author

ocelotl commented Jul 3, 2024

Ok, I am back to my original plan of just skipping this test case for pypy or windows. I also don't like to lower test coverage but there is no solution.

I tried to rerun this test case, it failed 3 times in a row: https://github.com/open-telemetry/opentelemetry-python/actions/runs/9782117611/job/27007817604?pr=3986#step:7:739

Also, trying to bump the delta is a naive approach: https://github.com/open-telemetry/opentelemetry-python/actions/runs/9782117611/job/27007817604?pr=3986#step:7:741
It failed with a delta of 2755! 🤦 3255.549192428589 != 500 within 40 delta (2755.549192428589 difference)

@ocelotl ocelotl changed the title Rerun test case when it fails Skip test case in pypy or windows Jul 3, 2024
@ocelotl
Copy link
Contributor Author

ocelotl commented Jul 3, 2024

I tried rerunning the test cases with a 1s delay between runs, still failed. I got this idea from pytest-retry which includes a delay option.

@ocelotl
Copy link
Contributor Author

ocelotl commented Jul 3, 2024

@xrmx please take a look, I tried rerunning the test case, it still fails. If we find a solution to make this test case run consistently in Windows, then we can add that solution in a subsequent PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

test_batch_span_processor_scheduled_delay failing in windows
3 participants