1248087 - screenshots: Add preferences and devtools scheduled runs

Status: RESOLVED FIXED

(Testing :: mozscreenshots, defect)

Description

[Matthew N. [:MattN]]

I think the following are useful enough to schedule for Nightlies:
* ["DevTools"]
* ["WindowSize", "Preferences"]

I'm going to move the current defaults to browser_primaryUI.js to simplify head.js so browser_screenshots.js exclusively handles when MOZSCREENSHOTS_SETS is set.

Comment 1

[Matthew N. [:MattN]]

Attachment: MozReview Request: Bug 1248087 - screenshots: Add preferences and devtools scheduled runs. r=kitcambridge

Review commit: https://reviewboard.mozilla.org/r/34861/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/34861/

Comment 2

[Lina Butler [:lina]]

Comment on attachment 8719067 [details]
MozReview Request: Bug 1248087 - screenshots: Add preferences and devtools scheduled runs. r=kitcambridge

https://reviewboard.mozilla.org/r/34861/#review31471

Looks good; just some questions for clarification.

::: browser/tools/mozscreenshots/browser.ini:5
(Diff revision 1)
> +tags = screenshots

I'm a bit unclear on the relationship between subsuites and tags. https://developer.mozilla.org/en-US/docs/Mochitest says "applying a subsuite to a test removes that test from the default set.  Whereas a tag will not remove it from the default set". What does it mean if you apply a tag to tests that are part of a subsuite?

::: browser/tools/mozscreenshots/browser_screenshots.js:13
(Diff revision 1)
> +  let setsEnv = env.get("MOZSCREENSHOTS_SETS");

Nit: Declaration could be moved above and used in the `if`.

::: browser/tools/mozscreenshots/head.js:28
(Diff revision 1)
> +  // Try pushes only capture in browser_screenshots.js with MOZSCREENSHOTS_SETS.

OK...with the changes in this patch:

* If `MOZSCREENSHOT_SETS` is set (locally or on Try), `browser_screenshots.js` runs them.

* `DevTools, WindowSize, Preferences` only run if we're building Nightly or locally, not on Try. (Where does the `AppConstants.SOURCE_REVISION_URL == "1"` check come in?)

Is that accurate?

Comment 3

[Matthew N. [:MattN]]

https://reviewboard.mozilla.org/r/34861/#review31471

> I'm a bit unclear on the relationship between subsuites and tags. https://developer.mozilla.org/en-US/docs/Mochitest says "applying a subsuite to a test removes that test from the default set.  Whereas a tag will not remove it from the default set". What does it mean if you apply a tag to tests that are part of a subsuite?

I don't actually remember and I also don't remember why I had this in the file before so I'll just remove it instead.

> Nit: Declaration could be moved above and used in the `if`.

Fixed

> OK...with the changes in this patch:
> 
> * If `MOZSCREENSHOT_SETS` is set (locally or on Try), `browser_screenshots.js` runs them.
> 
> * `DevTools, WindowSize, Preferences` only run if we're building Nightly or locally, not on Try. (Where does the `AppConstants.SOURCE_REVISION_URL == "1"` check come in?)
> 
> Is that accurate?

Oops, shouldCapture should return false for local builds where `MOZSCREENSHOT_SETS` is specified. I'll fix that.

The SOURCE_REVISION_URL == "1" is a workaround for a recent change (bug 1248027) while I see what is said there. I added a comment now.

Comment 4

[Matthew N. [:MattN]]

Comment on attachment 8719067 [details]
MozReview Request: Bug 1248087 - screenshots: Add preferences and devtools scheduled runs. r=kitcambridge

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/34861/diff/1-2/

Comment 5

[Lina Butler [:lina]]

Cool, this makes sense. LGTM!

Comment 6

[Pulsebot]

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

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/9ada33725ffd