Skip to content

Conversation

@orlitzky
Copy link
Contributor

In commit a181883 I made this test wait a bit longer (to be more robust under load) and added a "long time" to the corresponding start_workers() line. But, further down, there's another test that calls finish(), which needs the workers to be started. Without a "long time" on the finish(), it can fail sans --long.

To fix it, we combine the two tests that call start_workers() and finish(), and then mark the whole thing "long time". The finish() is supposed to delete the workers created in setup_workers() as well, so there's not much point in trying to make the "long time" more fine-grained than that.

In commit a181883 I made this test wait a bit longer (to be more
robust under load) and added a "long time" to the corresponding
start_workers() line. But, further down, there's another test that
calls finish(), which needs the workers to be started.  Without a
"long time" on the finish(), it can fail sans --long.

To fix it, we combine the two tests that call start_workers() and
finish(), and then mark the whole thing "long time". The finish() is
supposed to delete the workers created in setup_workers() as well, so
there's not much point in trying to make the "long time" more
fine-grained than that.
@github-actions
Copy link

Documentation preview for this PR (built with commit 94949db; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

@tornaria
Copy link
Contributor

Lgtm, I will test later.

@tobiasdiez
Copy link
Contributor

Let's mark it as CI fix so that it gets applied to the other PRs (where the meson workflow fails due to this problem).

@tobiasdiez tobiasdiez added the p: CI fix merged before running CI tests label Oct 29, 2024
@vbraun vbraun merged commit 34f93ae into sagemath:develop Nov 3, 2024
18 of 23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

p: CI fix merged before running CI tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants