931889 - Fix and re-enable browser_thumbnails_background.js

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Ed Morley [:emorley]]

browser_thumbnails_background.js is really unreliable, particularly on Linux. See dependent bugs.

As such, it has been disabled on Linux for now.

Comment 1

[Ed Morley [:emorley]]

Disabled on Linux by:
remote:   https://hg.mozilla.org/mozilla-central/rev/e7439c6c1e81

Comment 2

[Drew Willcoxon :adw]

Attachment: split 'r up

This splits up the test like we talked about.  It also:

* Modifies the bg tests to use helpers in head.js instead of their own where
  possible
* Adds helpers to head.js
* Modifies next() and TestRunner.next() in head.js to take arguments and pass
  them to the yielder via _iter.send()
* Drops the privateBrowsingActive and openPrivateWindowDuringCapture tests
  since I don't think they're useful anymore, but if you'd like, I can add
  them back

Comment 3

[Drew Willcoxon :adw]

iter.send(undefined) works on a newborn iterator, so this head.js change:

>       let value = aValue ? TestRunner._iter.send(aValue) :
>                   TestRunner._iter.next();

can be this instead, since TestRunner.run() calls this.next() with no arguments:

>       let value = TestRunner._iter.send(aValue);

I'll batch that change with any other comments you might have.

Comment 4

[Mark Hammond [:markh] [:mhammond]]

Comment on attachment 8334373 [details] [diff] [review]
split 'r up

Review of attachment 8334373 [details] [diff] [review]:
-----------------------------------------------------------------

looks good, sorry for the earlier confusion.

::: toolkit/components/thumbnails/test/browser_thumbnails_bg_queueing.js
@@ +9,5 @@
> +    bgTestPageURL({ wait: 2002 }),
> +    "http://www.example.com/2",
> +  ];
> +  urls.forEach(u => ok(!thumbnailExists(u), "Thumbnail should not exist yet"));
> +  urls.forEach(function (url) {

might as well use a fat-arrow callback here too

::: toolkit/components/thumbnails/test/head.js
@@ +54,2 @@
>      try {
> +      let value = aValue ? TestRunner._iter.send(aValue) :

yeah, the simpler version you mentioned would be better.

Comment 5

[Drew Willcoxon :adw]

https://tbpl.mozilla.org/?tree=Try&rev=6d92aa1ec693

Comment 6

[Drew Willcoxon :adw]

Attachment: split 'r up (for check-in)

Tryserver didn't show any oranges that weren't already there, so this is ready to land.  After that, I think this bug can be closed and new bugs filed against any new failing tests if necessary.

Comment 7

[Drew Willcoxon :adw]

Er, I forgot about the many dependent bugs.  I think each can be rescoped to whatever test file they now happen to be failing in.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/c189beb8d5f5

Comment 9

[Ryan VanderMeulen [:RyanVM]]

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

Comment 10

[Ada [:adalucinet]]

Is there any help needed for manual testing?

Comment 11

[Drew Willcoxon :adw]

No.