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

More attempts to get CI to be less flaky #18652

Closed
wants to merge 6 commits into from

Conversation

benjyw
Copy link
Contributor

@benjyw benjyw commented Apr 1, 2023

Based on tests that still fail in a loaded CI environment, even after #18556

@benjyw benjyw added the category:internal CI, fixes for not-yet-released features, etc. label Apr 1, 2023
@benjyw benjyw requested review from stuhood and kaos April 1, 2023 21:59
@benjyw
Copy link
Contributor Author

benjyw commented Apr 1, 2023

Harrumph, this test failure is not at all clear to me: https://github.com/pantsbuild/pants/actions/runs/4585391355/jobs/8097532089?pr=18652

@stuhood could this be a byproduct of #18632? Or something it should have fixed but didn't?

Copy link
Member

@kaos kaos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using arbitrary delays with time.sleep() is very unfortunate.

Can we come up with a more predictable way to synchronize the condition we are actually waiting for?

@benjyw benjyw force-pushed the bump_a_test_timing branch from 5b4b5f8 to 3b90a21 Compare April 2, 2023 15:18
@benjyw benjyw force-pushed the bump_a_test_timing branch 5 times, most recently from f8d6c7b to b3774de Compare April 3, 2023 04:04
@benjyw benjyw force-pushed the bump_a_test_timing branch from b3774de to f12c805 Compare April 3, 2023 05:24
@stuhood
Copy link
Member

stuhood commented Apr 3, 2023

@stuhood could this be a byproduct of #18632? Or something it should have fixed but didn't?

No... I think that it is more likely to be due to #18153. Filed #18661 for it.

Comment on lines +431 to 437
# Ensure the test invocation of Pants doesn't restart itself after it writes the tailored files,
# and then chokes on the invalid BUILD symbol.
result = rule_runner.run_goal_rule(
TailorGoal, args=["--alias-mapping={'fortran_library': 'my_fortran_lib'}", "::"]
TailorGoal,
global_args=["--no-watch-filesystem", "--no-pantsd"],
args=["--tailor-alias-mapping={'fortran_library': 'my_fortran_lib'}", "::"],
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not an integration test, so no external processes or pantsd are spawned: everything runs in process, and file invalidation happens synchronously during the write_files call. So these arguments have no effect.

@@ -355,7 +355,7 @@ def test_pantsd_invalidation_file_tracking(self):
ctx.checker.assert_started()

# See comment in `test_pantsd_invalidation_pants_toml_file`.
time.sleep(15)
time.sleep(30)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made a recent attempt at bumping these, without luck.

It might be a good idea to assign me a ticket to more deeply investigate these, and then skip them for now.

@benjyw benjyw force-pushed the bump_a_test_timing branch from cf0b9dc to cc071d3 Compare April 3, 2023 20:45
@benjyw benjyw closed this Apr 3, 2023
@benjyw benjyw deleted the bump_a_test_timing branch April 3, 2023 21:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:internal CI, fixes for not-yet-released features, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants