Skip to content

Conversation

@thecrypticace
Copy link
Contributor

@thecrypticace thecrypticace commented Nov 4, 2024

This fixes a potential race condition with watcher cleanup but that was not the source of the flakiness.

We discovered that if Parcel watcher was subscribed to a directory and a child directory it could result in events not firing — even for different directories. Oddly, this only seemed to happen if we:

  • Scanned a directory from oxide
  • Passed the returned candidates through to compile(…).build(…)
  • And subsquently wrote that result to the output CSS file

However, if we hardcoded the list of candidates passed to build(…) regardless of whether or not Oxide scanned the filesystem for candidates the issue did not occur (though maybe it was just less reproducible — this was after all a VERY flaky test).

Given that watching children of an already watched directory is doing unnecessary work, we decided to cleanup the list of watched directories such that this does not happen. It appears to fix the flakiness but we'll want to keep an eye on it in case the flakiness returns.

RobinMalfait and others added 11 commits November 3, 2024 02:12
Unsubscribing from a watcher returns a promise, which we didn't wait
for. This makes the `Disposables#dispose` method async to ensure we
properly wait.
This is a speculative fix for CI failures on macOS
@thecrypticace thecrypticace marked this pull request as ready for review November 4, 2024 17:56
@thecrypticace thecrypticace requested a review from a team as a code owner November 4, 2024 17:56
@thecrypticace thecrypticace changed the title Fix macOS test flakiness (hopefully) Fix macOS test flakiness Nov 4, 2024
@thecrypticace thecrypticace merged commit 5f3630b into next Nov 4, 2024
1 check passed
@thecrypticace thecrypticace deleted the fix/standalone-cli-watch-mode branch November 4, 2024 18:07
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 this pull request may close these issues.

3 participants