Skip to content

Async BenchmarkRunner and removing BenchmarkState#27

Merged
camillobruni merged 5 commits intoWebKit:mainfrom
camillobruni:2022-11-22_async
Nov 23, 2022
Merged

Async BenchmarkRunner and removing BenchmarkState#27
camillobruni merged 5 commits intoWebKit:mainfrom
camillobruni:2022-11-22_async

Conversation

@camillobruni
Copy link
Contributor

@camillobruni camillobruni commented Nov 22, 2022

Replace the continuation-based test running with an async-await loop over all suites and tests.
This also removes the necessity of the BenchmarkState class.

this._finalize();
return Promise.resolve();
}
this._removeFrame();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to call this._removeFrame() here??

Copy link
Contributor Author

@camillobruni camillobruni Nov 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm just cargo-culting here, and moved it out of the _finalize method.
But it turns that we should remove it here to get a clean view for displaying the results page (added a comment to clarify this).

Copy link
Member

@rniwa rniwa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r=me

@camillobruni
Copy link
Contributor Author

thanks! I will inject an intermediate PR that will move the functions to a more chronological oder for PR #26.

@camillobruni camillobruni merged commit 731c9e8 into WebKit:main Nov 23, 2022
@camillobruni camillobruni deleted the 2022-11-22_async branch November 30, 2022 18:29
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