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

Avoid race between visit tests #310

Merged
merged 1 commit into from
Jul 14, 2021
Merged

Conversation

ctrochalakis
Copy link
Contributor

@ctrochalakis ctrochalakis commented Jul 14, 2021

TurboDriveTestCase maintains a eventLogChannel that acts as a cursor
for the page's window.eventLogs array. Before every test the
eventLogChannel is drained by reading all events and its internal
index (cursor) is updated accordingly.

The race was happening when we changed the page location and not
blocking on it: the eventLogChannel was draining the previous page
bumping its internal index. After the page was reload that index was
out of sync since the new page had a new, empty eventLogs array.

Order of events

  • async location change
  • draining eventLogChannel on the previous page
  • actual location change
  • eventLogChannel is out-of-sync

This was fixed as part of #289

TurboDriveTestCase maintains a eventLogChannel that acts as a cursor
for the page's window.eventLogs array. Before every test the
eventLogChannel is drained by reading all events and its internal
index (cursor) is updated accordingly.

The race was happening when we changed the page location and not
blocking on it: the eventLogChannel was draining the previous page
bumping its internal index. After the page was reload that index was
out of sync since the new page had an new, empty eventLogs array.

Order of events
 * async location change
 * draining eventLogChannel on the previous page
 * actual location change
 * eventLogChannel is out-of-sync

This is was fixed as part of hotwired#289
@dhh dhh merged commit 9dfca8f into hotwired:main Jul 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants