1665645 - "editor/libeditor/tests/test_CF_HTML_clipboard.html" misses `requestFlakyTimeout` call

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect)

Description

[Mirko Brodesser (:mbrodesser-Igalia), offline until April 25h]

E.g., "editor/libeditor/tests/test_CF_HTML_clipboard.html" is never executed on Try.

It calls setTimeout without requesting a flaky timeout. Hence it should fail (and does, locally).

It's intended to be run only on Mac. However, because it doesn't show up on Try runs, it means it's never executed there.

This might affect other tests like these too.

Comment 1

[Geoff Brown [:gbrown]]

(In reply to Mirko Brodesser (:mbrodesser) from comment #0)

E.g., "editor/libeditor/tests/test_CF_HTML_clipboard.html" is never executed on Try.

It will be executed on try if Mac mochitests are requested:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=68c1395467bd706ca03282f4c14be30e10cd648e

Sorry if I am being too literal...I would like to see the issue well defined. Is it that 'mach try auto' didn't run tests on osx, or ...?

Comment 2

[Sarah Clements [:sclements]]

This doesn't sound like a Treeherder issue. :gbrown, do you know what component this should be moved to?

Comment 3

[Mirko Brodesser (:mbrodesser-Igalia), offline until April 25h]

(In reply to Geoff Brown [:gbrown] from comment #1)

(In reply to Mirko Brodesser (:mbrodesser) from comment #0)

E.g., "editor/libeditor/tests/test_CF_HTML_clipboard.html" is never executed on Try.

It will be executed on try if Mac mochitests are requested:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=68c1395467bd706ca03282f4c14be30e10cd648e

The try run seems to not contain an execution of "editor/libeditor/tests/test_CF_HTML_clipboard.html". The error summary contains only "editor/spellchecker/tests/mochitest.ini" when searching for "editor". Which excludes "editor/libeditor/tests/test_CF_HTML_clipboard.html".

Sorry if I am being too literal...I would like to see the issue well defined. Is it that 'mach try auto' didn't run tests on osx, or ...?

Thanks for being precise :-). The issue is, I'd expect the test to fail on Treeherder, see c0. But when looking at arbitrary Treeherder runs (e.g. this one) it's not shown as a failure and AFAICT, it wasn't run, but it should've been.
Now I see that not distinguishing between Try runs and other Treeherder runs might've caused confusion, sorry.

Comment 4

[Geoff Brown [:gbrown]]

My try run does contain an execution of that test, in M(4) for both opt and debug.

https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=315967665&repo=try&lineNumber=8372

[task 2020-09-17T15:52:29.447Z] 15:52:29     INFO - TEST-START | editor/libeditor/tests/test_CF_HTML_clipboard.html
[task 2020-09-17T15:52:30.749Z] 15:52:30     INFO - GECKO(1910) | MEMORY STAT vsizeMaxContiguous not supported in this build configuration.
[task 2020-09-17T15:52:30.749Z] 15:52:30     INFO - GECKO(1910) | MEMORY STAT | vsize 6552MB | residentFast 84MB | heapAllocated 13MB
[task 2020-09-17T15:52:30.762Z] 15:52:30     INFO - TEST-OK | editor/libeditor/tests/test_CF_HTML_clipboard.html | took 1322ms

Comment 5

[Mirko Brodesser (:mbrodesser-Igalia), offline until April 25h]

(In reply to Geoff Brown [:gbrown] from comment #4)

My try run does contain an execution of that test, in M(4) for both opt and debug.

https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=315967665&repo=try&lineNumber=8372

[task 2020-09-17T15:52:29.447Z] 15:52:29     INFO - TEST-START | editor/libeditor/tests/test_CF_HTML_clipboard.html
[task 2020-09-17T15:52:30.749Z] 15:52:30     INFO - GECKO(1910) | MEMORY STAT vsizeMaxContiguous not supported in this build configuration.
[task 2020-09-17T15:52:30.749Z] 15:52:30     INFO - GECKO(1910) | MEMORY STAT | vsize 6552MB | residentFast 84MB | heapAllocated 13MB
[task 2020-09-17T15:52:30.762Z] 15:52:30     INFO - TEST-OK | editor/libeditor/tests/test_CF_HTML_clipboard.html | took 1322ms

Thanks for pointing this out. Then, I wonder why the test doesn't fail, because it doesn't request a flaky timeout but calls setTimeout.

Requesting a flaky timeout can be done only via calling requestFlakyTimeout. According to a comment and the nearby code it should be enabled for mochitest-plain, to which the above mentioned test belongs.

Or do I miss something?

Comment 6

[Geoff Brown [:gbrown]]

That is interesting!

I added some debugging on try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c758c3536c157864dcbe324f41532861e7d7e555

The setTimeout with time=0 is called (exempt from flaky checks):
https://searchfox.org/mozilla-central/rev/d410917833190ee08a62b38f8f9de65cea1e661b/editor/libeditor/tests/test_CF_HTML_clipboard.html#125

But none of the other setTimeout calls are executed, as far as I can see.

Comment 7

[Mirko Brodesser (:mbrodesser-Igalia), offline until April 25h]

(In reply to Geoff Brown [:gbrown] from comment #6)

That is interesting!

I added some debugging on try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c758c3536c157864dcbe324f41532861e7d7e555

The setTimeout with time=0 is called (exempt from flaky checks):
https://searchfox.org/mozilla-central/rev/d410917833190ee08a62b38f8f9de65cea1e661b/editor/libeditor/tests/test_CF_HTML_clipboard.html#125

But none of the other setTimeout calls are executed, as far as I can see.

Yes, thanks for try run and the analysis.

After some more analysis, it seems correct, that not all setTimeout calls are actually executed. The functionality depends on platform-specific code, hence timings can differ. Explanation:

The clipboard is filled here. copyCF_HTML_worker is executed immediately afterwards. If the clipboard already contains the data, the first setTimeout call is not executed, which is OK. If getting the data from the transferable succeeds, the second setTimeout call is not executed, which is OK.

To make the test more robust, requestFlakyTimeout should be called. I'll add a review for this.

Comment 8

[Mirko Brodesser (:mbrodesser-Igalia), offline until April 25h]

Attachment: Bug 1665645: add missing `requestFlakyTimeout` call to "editor/libeditor/tests/test_CF_HTML_clipboard.html". r=hsivonen

See https://bugzilla.mozilla.org/show_bug.cgi?id=1665645#c7 for more
context.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e748d899b8df
add missing `requestFlakyTimeout` call to "editor/libeditor/tests/test_CF_HTML_clipboard.html". r=hsivonen

Comment 10

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/e748d899b8df