WebRTWebRtcAudioDebugRecordingsBrowserTest failing on WebRTC windows bots |
||||||
Issue descriptionSample logs: ../../content/browser/webrtc/webrtc_audio_debug_recordings_browsertest.cc(150): error: Value of: base::DeleteFile(file_path, false) Actual: false Expected: true ../../content/browser/webrtc/webrtc_audio_debug_recordings_browsertest.cc(174): error: Value of: base::IsDirectoryEmpty(temp_dir_path) Actual: false Expected: true ../../content/browser/webrtc/webrtc_audio_debug_recordings_browsertest.cc(175): error: Value of: base::DeleteFile(temp_dir_path, false) Actual: false Expected: true The the files are not deleted in the test could be that all the side effects of MediaStreamTrack.stop() are not complete yet at deletion time and this somehow prevents the files to be deleted. Unfortunately MediaStreamTrack.stop() does not have a callback to get a notification of when all its side effects are complete, so maybe the fix is to just wait some time before trying to delete the files.
,
Nov 10 2017
,
Nov 14 2017
,
Nov 24 2017
Guido: which bot(s) was this? Is the test disabled now? And this is a regression, right? Which build did it start with?
,
Nov 24 2017
All WebRTC windows bots. The problem is that MediaStreamTrack.stop()'s implementation is asynchronous even though its API suggests it's not (it has no callbacks/promises). This was causing some races with getUserMedia(), which were fixed by serializing stop() requests together with getUserMedia() to make sure that side effects from ech stop() call are complete before processing the next stop() or getUserMedia() request. I believe the extra delays introduced by this serialization made this test fail on Windows WebRTC bots. I made some fixes to minimize the delay in most cases, so it might be worth it to just re-enable the test on Windows to see if it starts passing. Otherwise, just sleeping a little bit after all the stop calls should fix it too.
,
Nov 24 2017
Thanks for the explanation! OK, I'll see what can be done.
,
Dec 5 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/150d636613446a107edee5809a738f17ca80e9a4 commit 150d636613446a107edee5809a738f17ca80e9a4 Author: Henrik Grunell <grunell@chromium.org> Date: Tue Dec 05 14:30:10 2017 Re-enable WebRtcAudioDebugRecordingsBrowserTest.CallWithAudioDebugRecordings on Windows. Speculatively re-enable based on comment #5 in the bug. TBR=guidou@chromium.org Bug: 783790 Change-Id: Ie10aa49212a4279a8696dfbc021a577bda0a2dfd Reviewed-on: https://chromium-review.googlesource.com/808845 Reviewed-by: Henrik Grunell <grunell@chromium.org> Commit-Queue: Henrik Grunell <grunell@chromium.org> Cr-Commit-Position: refs/heads/master@{#521686} [modify] https://crrev.com/150d636613446a107edee5809a738f17ca80e9a4/content/browser/webrtc/webrtc_audio_debug_recordings_browsertest.cc
,
Dec 6 2017
Still flaky on Windows after re-enabling: https://test-results.appspot.com/dashboards/flakiness_dashboard.html#testType=content_browsertests&tests=WebRtcAudioDebugRecordingsBrowserTest.CallWithAudioDebugRecordings Though, in all runs so far the retries have passed. Simplest solution would be a retry in the test, with a little delay. That's probably good enough. Alternatively, the test could listen to some internal state but afaik (haven't looked yet) that requires code changes for this purpose only which is to be avoided.
,
Dec 7 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4b3b8130b5df559b562a43b28298089f8b9bf15d commit 4b3b8130b5df559b562a43b28298089f8b9bf15d Author: Henrik Grunell <grunell@chromium.org> Date: Thu Dec 07 16:24:43 2017 Move AEC dump verification to last in WebRTWebRtcAudioDebugRecordingsBrowserTest tests. An attempt to reduce flakiness, based on that the AEC dump requires longer time before being closed than the input and output recordings due to IPC and more thread jumps. See also the added comment in the file for more info. Bug: 783790 Change-Id: I1f2cd838a749eb1ec76e6e32e283b26e89ff293e Reviewed-on: https://chromium-review.googlesource.com/813715 Commit-Queue: Henrik Grunell <grunell@chromium.org> Reviewed-by: Guido Urdaneta <guidou@chromium.org> Cr-Commit-Position: refs/heads/master@{#522445} [modify] https://crrev.com/4b3b8130b5df559b562a43b28298089f8b9bf15d/content/browser/webrtc/webrtc_audio_debug_recordings_browsertest.cc
,
Dec 11 2017
Nah, #9 didn't help. So, I'll introduce a retry after a little delay if delete file fails. The flakiness affects the other test cases too, so the retry should be in all of them.
,
Jan 9 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2cced9918b45eba08a8035cdbefba96e754e8b11 commit 2cced9918b45eba08a8035cdbefba96e754e8b11 Author: Henrik Grunell <grunell@chromium.org> Date: Tue Jan 09 10:40:19 2018 Retry deleting file in WebRtcAudioDebugRecordingsBrowserTest. This is a simple way to mitigate flaky failure due to racy file close vs. delete. Bug: 783790 Change-Id: I01a71399b16fd392c5509f851cc1f4ec5b21f750 Reviewed-on: https://chromium-review.googlesource.com/832465 Commit-Queue: Henrik Grunell <grunell@chromium.org> Reviewed-by: Guido Urdaneta <guidou@chromium.org> Cr-Commit-Position: refs/heads/master@{#527957} [modify] https://crrev.com/2cced9918b45eba08a8035cdbefba96e754e8b11/content/browser/webrtc/webrtc_audio_debug_recordings_browsertest.cc
,
Jan 9 2018
Will check flakiness dashboard in a few days. For reference, it still is flaky.
,
Jan 12 2018
Looks good now. |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by guidou@chromium.org
, Nov 10 2017