Tweaks to WASM CI for sparse strips#1680
Merged
Merged
Conversation
Comment on lines
+326
to
+328
| # We use MacOS because for some reason the Ubuntu-based runners | ||
| # have trouble handling things like `requestAnimationFrame` or | ||
| # `setTimeout` in our testing code. |
Contributor
There was a problem hiding this comment.
This magic is quite unfortunate but I agree that being pragmatic makes sense and it's not worth burning all the time on this.
Collaborator
Author
|
@raphlinus @DJMcNab I think I will need one of you again to merge this to circumvent the bug where CI endlessly waits for a check that isn't going to happen 😦 |
Member
|
That's not a bug, it's a feature. The old test job is marked as mandatory and removing it from the CI script doesn't remove that requirement. Anyway, I removed that requirement and will mark these new ones as required. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR introduces two changes to our CI for testing sparse strips in CI:
Instead of having one single job, we split them up into three jobs: The first job for checking the examples, the second for running the tests without WASM and the third for running the tests with WASM. This cuts the "bottleneck" time for this test specifically from around 13 minutes to around 6-7 minutes. This won't have an immediate effect since the overall bottleneck is still Windows CI, but it's an easy change and will likely be worth it in the future once we reduce the time of Windows CI.
Use MacOS-based runners instead of Ubuntu for running the WASM tests. When working on vello_hybrid: Make probe more asynchronous #1671, I have encountered the problem that the probe test does not pass anymore due to some weird timeout.
I've spent more than a day trying to debug this now, but have not gotten further than concluding that for some reason, invoking asynchronous methods like
setTimeoutorrequestAnimationFramecauses those timeouts to occur, regardless of what happens in there (even when removing the actual probing, it still happened when I invoked those methods). Locally everything works fine, and apparently it also works fine when switching to a MacOS-based runners. I'm fairly certain that this is some weirdness with the Github CI runners and not with probing failing completely on Linux (though I admittedly haven't tested it since I don't have a linux laptop), so I think this is the easiest thing to do for now. I'd also like to get to the bottom of this, but I've already spent more time on this than I can really justify, so I hope that this is fine doing for now.