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

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

Conversation

@cdumez
Copy link
Contributor

@cdumez cdumez commented Apr 9, 2021

…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.

…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 Apr 9, 2021
webkit-commit-queue pushed a commit to WebKit/WebKit that referenced this pull request Apr 10, 2021
…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 cdumez commented Apr 10, 2021

@rtoy
rtoy approved these changes Apr 12, 2021
Copy link

@rtoy rtoy left a comment

Thanks for fixing this!

@cdumez cdumez merged commit ab5a361 into web-platform-tests:master Apr 12, 2021
19 checks passed
19 checks passed
@azure-pipelines
Azure Pipelines Build #20210409.19 succeeded
Details
@azure-pipelines
Azure Pipelines (./wpt test-jobs) ./wpt test-jobs succeeded
Details
@azure-pipelines
Azure Pipelines (affected tests without changes: Safari Technology Preview) affected tests without changes: Safari Technology Preview succeeded
Details
@azure-pipelines
Azure Pipelines (affected tests: Safari Technology Preview) affected tests: Safari Technology Preview succeeded
Details
@azure-pipelines
Azure Pipelines (wpt.fyi hook: safari-preview-affected-tests) wpt.fyi hook: safari-preview-affected-tests succeeded
Details
@azure-pipelines
Azure Pipelines (wpt.fyi hook: safari-preview-affected-tests-without-changes) wpt.fyi hook: safari-preview-affected-tests-without-changes succeeded
Details
@community-tc-integration
download-firefox-nightly Community-TC (pull_request)
Details
@community-tc-integration
lint Community-TC (pull_request)
Details
@community-tc-integration
sink-task Community-TC (pull_request)
Details
@community-tc-integration
wpt-chrome-dev-results Community-TC (pull_request)
Details
@community-tc-integration
wpt-chrome-dev-results-without-changes Community-TC (pull_request)
Details
@community-tc-integration
wpt-chrome-dev-stability Community-TC (pull_request)
Details
@community-tc-integration
wpt-decision-task Community-TC (pull_request)
Details
@community-tc-integration
wpt-firefox-nightly-results Community-TC (pull_request)
Details
@community-tc-integration
wpt-firefox-nightly-results-without-changes Community-TC (pull_request)
Details
@community-tc-integration
wpt-firefox-nightly-stability Community-TC (pull_request)
Details
@wpt-fyi
wpt.fyi - chrome[experimental] Chrome results
Details
@wpt-fyi
wpt.fyi - firefox[experimental] Firefox results
Details
@wpt-fyi
wpt.fyi - safari[experimental] Safari results
Details
yury-s pushed a commit to yury-s/webkit that referenced this pull request Apr 12, 2021
…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 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 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 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
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants