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

[benchmarks] Fix edit and "after edit" experiments #2122

Merged
merged 1 commit into from
Aug 22, 2021

Conversation

pepeiborra
Copy link
Collaborator

When the example includes >1 modules (like the Cabal example), this is the sequence of events:

--> didChange A.hs
--> didChange B.hs
<-- [ProgressDone (abort the previous progress, if any)]
<-- ProgressStart (for A.hs change)
<-- ProgressDone (aborted)
<-- ProgressStart (for B.hs change)
<-- ProgressDone

The edit and "after edit" experiments usually want to wait until the final set of diagnostics is produced. Currently the edit experiment waits for one progress start and one progress done, while the "after edit" experiments wait for one progress done. This is only correct if there is only one module in the example. When there are two or more, as seen above, it's not enough. The experiment needs to ignore the aborted progress done events and wait until the last progress done event.

This PR makes changes to wait for one progress start event per module and then wait for one progress done event after changing the last module.

When the example includes >1 modules, we have to wait for the progress
start of every edit. This is the sequence of events:

--> didChange A.hs
--> didChange B.hs
<-- ProgressStart (for A.hs change)
<-- ProgressDone (aborted)
<-- ProgressStart (for B.hs change)
<-- ProgressDone

The experiment needs to ignore the aborted progress done events and wait until the last progress done event.
To accomplish this, we wait for all the progress start events and then
wait for one progress done event
@pepeiborra pepeiborra merged commit e573c41 into master Aug 22, 2021
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.

2 participants