Errors shouldn't be ignored in mochitest except very specific cases

RESOLVED FIXED in Firefox 55

Status

()

defect
P1
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: jya, Assigned: ayang)

Tracking

(Blocks 1 bug)

unspecified
mozilla55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox55 fixed)

Details

Attachments

(1 attachment)

Reporter

Description

2 years ago
Bug 1361984 introduce a serious regression that broke YouTube on Windows.

Two problems occurred:
1- The issue could have been easily identified by manually testing the change.
2- regression tests also didn't find any problem either.

Unfortunately, such manual testing didn't occur, that made us relying on our regression tests to work as intended.

The issue didn't occur because by default, our media player ignores any errors and skip over them until it find something. While it doesn't cause an error, there are usually visually noticeable problems.

media.audio-max-decode-error and media.audio-max-decode-error should be set to 0 by default with the exception of a very particular mochitest that checks that those preferences do work as intended.

A decode error of any of our mochitest media files should yield to a test failure.
Reporter

Updated

2 years ago
Blocks: 1366195
Alfredo,
Our tolerance for decode errors in bug 1068151 might need to be changed.  
WDYT? Can you help check this bug?
Flags: needinfo?(ayang)
See Also: → 1068151
Reporter

Comment 2

2 years ago
I think they should be set to 0 during mochitest, except for the test that ensure we skip over errors if there's one (I don't see any which is a shame).
Reporter

Updated

2 years ago
See Also: → 1366362
Assignee

Updated

2 years ago
Assignee: nobody → ayang
Flags: needinfo?(ayang)
Assignee

Comment 3

2 years ago
(In reply to Jean-Yves Avenard [:jya] from comment #2)
> I think they should be set to 0 during mochitest, except for the test that

Or always set to zero in debug?
Reporter

Comment 4

2 years ago
That would be good too. Great idea. 

I still think it should be 0 during mochitest even on opt build.
Assignee

Comment 5

2 years ago
Ok, I'll go for debug build and the mochitest especially for the test like test_playback.html on opt build.
Comment hidden (mozreview-request)
Reporter

Comment 7

2 years ago
mozreview-review
Comment on attachment 8871194 [details]
Bug 1364872 - take zero tolerance of decoding error in debug and playback test.

https://reviewboard.mozilla.org/r/142680/#review146378

please add a mochitest with a file that does produce a decode error, to ensure that this functionality does wwork when we want to.
Attachment #8871194 - Flags: review?(jyavenard) → review+
Assignee

Comment 8

2 years ago
(In reply to Jean-Yves Avenard [:jya] from comment #7)
> Comment on attachment 8871194 [details]
> Bug 1364872 - take zero tolerance of decoding error in debug and playback
> test.
> 
> https://reviewboard.mozilla.org/r/142680/#review146378
> 
> please add a mochitest with a file that does produce a decode error, to
> ensure that this functionality does wwork when we want to.

I tried before, the test file is too large to be in gecko. I also tried to produce a problem video but it decodes to small mosaic artifical frames instead of errors.

So far I found is audio file in bug 1270748 but it doesn't cover video playback.
Reporter

Comment 9

2 years ago
Why didn't you commit bug 1270748 ?

For generating a broken video, you could use MSE and modify the buffer with rubbish so that it can be demuxed but the decoder will choke on it.
Assignee

Comment 10

2 years ago
(In reply to Jean-Yves Avenard [:jya] from comment #9)
> Why didn't you commit bug 1270748 ?
> 
> For generating a broken video, you could use MSE and modify the buffer with
> rubbish so that it can be demuxed but the decoder will choke on it.

Because I want to find another video testcase, not just audio. I'll try MSE for that.
Assignee

Comment 11

2 years ago
hmm... try fails at browser_styleeditor_media_sidebar.js but I really don't think it is my fault.


https://treeherder.mozilla.org/#/jobs?repo=try&revision=3a1ee3b5c0c73356ac719fb58818a6d6c3e7131e&selectedJob=102741961

Comment 12

2 years ago
Pushed by ayang@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1de90f1305e0
take zero tolerance of decoding error in debug and playback test. r=jya

Comment 13

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/1de90f1305e0
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.