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

Jump to bottom

realtimeanalyser-fft-scaling.html webaudio test is flaky in WebKit te… #28428

Merged
merged 1 commit into from Apr 12, 2021
Merged

realtimeanalyser-fft-scaling.html webaudio test is flaky in WebKit te… #28428

merged 1 commit into from Apr 12, 2021

Conversation

cdumez
Copy link
Contributor

@cdumez cdumez commented Apr 9, 2021 •
edited

…st suite

The subtests in the tests were running in parallel without any kind of synchronization. There was therefore
no guarantee about the order the PASS lines would be printed in. This led to flakiness in the WebKit test
suite: https://bugs.webkit.org/show_bug.cgi?id=223966.

To address the issue, we now run subtests one after another, in a well-defined order.

@cdumez
realtimeanalyser-fft-scaling.html webaudio test is flaky in WebKit te…
83268a1 
…st suite

The subtests in the tests were running in parallel without any kind of synchronization. There was therefore
no guarantee about the order the PASS lines would be printed in. This led to flakiness in the WebKit test
suite: https://bugs.webkit.org/show_bug.cgi?id=224377.

To address the issue, we now run subtests one after another, in a well-defined order.
@wpt-pr-bot wpt-pr-bot requested review from hoch, padenot and rtoy April 9, 2021 20:22
webkit-commit-queue pushed a commit to WebKit/WebKit that referenced this pull request Apr 10, 2021
@cdumez
[ macOS ] imported/w3c/web-platform-tests/webaudio/the-audio-api/the-…
63a588e 
…analysernode-interface/realtimeanalyser-fft-scaling.html is a flakey text failure

https://bugs.webkit.org/show_bug.cgi?id=223966
<rdar://problem/76028345>

Reviewed by Darin Adler.

LayoutTests/imported/w3c:

Update test as per:
- web-platform-tests/wpt#28428

Make sure subtests are run one after another, not in parallel, so that PASS lines are printed out in a
consistent order.

* web-platform-tests/webaudio/the-audio-api/the-analysernode-interface/realtimeanalyser-fft-scaling.html:

LayoutTests:

Unskip test that should no longer be flaky.

* platform/mac/TestExpectations:

Canonical link: https://commits.webkit.org/236374@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@275803 268f45cc-cd09-0410-ab3c-d52691b4dbfc
@cdumez
Copy link
Contributor Author

cdumez commented Apr 10, 2021

cc @rtoy @hoch

rtoy
rtoy approved these changes Apr 12, 2021
View reviewed changes
Copy link

@rtoy rtoy left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this!

@cdumez cdumez merged commit ab5a361 into web-platform-tests:master Apr 12, 2021
yury-s pushed a commit to yury-s/webkit-http that referenced this pull request Apr 12, 2021
[ macOS ] imported/w3c/web-platform-tests/webaudio/the-audio-api/the-…
de19b77 
…analysernode-interface/realtimeanalyser-fft-scaling.html is a flakey text failure

https://bugs.webkit.org/show_bug.cgi?id=223966
<rdar://problem/76028345>

Reviewed by Darin Adler.

LayoutTests/imported/w3c:

Update test as per:
- web-platform-tests/wpt#28428

Make sure subtests are run one after another, not in parallel, so that PASS lines are printed out in a
consistent order.

* web-platform-tests/webaudio/the-audio-api/the-analysernode-interface/realtimeanalyser-fft-scaling.html:

LayoutTests:

Unskip test that should no longer be flaky.

* platform/mac/TestExpectations:

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@275803 268f45cc-cd09-0410-ab3c-d52691b4dbfc
@karlt
Copy link
Contributor

karlt commented Apr 13, 2021

I assume https://bugs.webkit.org/show_bug.cgi?id=223966 was the intended WebKit bug.

With async_test(), "Keep in mind that other tests could start executing before an Asynchronous Test is finished", so I'd expect the harness to have similar issues in many tests. I assume other test harnesses are not particular about the order of results, though inconsistent results could still show up if there is a crash.

audit.js uses promise_test() so usually runs tasks in order. The parallel nature of this test seems to be specific to the way Audit was used here.

@cdumez
Copy link
Contributor Author

cdumez commented Apr 13, 2021

I assume https://bugs.webkit.org/show_bug.cgi?id=223966 was the intended WebKit bug.

Yes, sorry about that.

With async_test(), "Keep in mind that other tests could start executing before an Asynchronous Test is finished", so I'd expect the harness to have similar issues in many tests. I assume other test harnesses are not particular about the order of results, though inconsistent results could still show up if there is a crash.

audit.js uses promise_test() so usually runs tasks in order. The parallel nature of this test seems to be specific to the way Audit was used here.

Yes, I do believe the issue was specific to this test (or a few tests). This is not the first time I have to fix webaudio tests to get reliable ordering: see dfd29f2.

@rtoy
Copy link

rtoy commented Apr 14, 2021

Sorry about that. Since Chrome doesn't use expected files anymore (except when needed), I didn't notice that the order could change. All the tests pass, so the order didn't matter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rtoy rtoy rtoy approved these changes

@hoch hoch Awaiting requested review from hoch

@padenot padenot Awaiting requested review from padenot

Assignees

@rtoy rtoy

Labels
webaudio wg-audio
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@cdumez @karlt @rtoy @wpt-pr-bot