1169179 - (mozscreenshots) Run mozscreenshots via mochitest-browser-chrome

Status: RESOLVED FIXED

(Testing :: mozscreenshots, enhancement)

Description

[Matthew N. [:MattN]]

Attachment: v0.1 Add browser_screenshots.js in the screenshots subsuite

Being able to run mozscreenshots from mach and in automation will make it much more convenient to use and therefore likely to be used more often to detect visual regressions and to perform UI reviews while features are in development.

I think running it via the mochitest-browser-chrome harness is probably easiest and I think it can be a subsuite so it won't run by default but can have it's own buildbot job type in automation triggered by try syntax.

Some things to figure out:
* Where to get the XPI from (e.g. build in m-c or fetch from another repo?)
* Where the test file should live

Once we have this figured out I can file follow-ups for getting this running in automation.

Comment 1

[Matthew N. [:MattN]]

Example try syntax with this patch applied:
`
try: -b o -p linux64,macosx64,win32,win64 -u mochitest-bc[Ubuntu,10.6,10.10,Windows XP,Windows 7,Windows 8] -t none --subsuite screenshots --setenv MOZSCREENSHOTS_SETS=DevEdition,TabsInTitlebar,Tabs,WindowSize,Toolbars
`

Comment 2

[Matthew N. [:MattN]]

Attachment: MozReview Request: Bug 1169179 - Run mozscreenshots as a mochitest-browser-chrome test. r=felipe,glandium

Bug 1169179 - Run mozscreenshots as a mochitest-browser-chrome test. r=felipe,glandium

Comment 3

[Matthew N. [:MattN]]

Comment on attachment 8697332 [details]
MozReview Request: Bug 1169179 - Run mozscreenshots as a mochitest-browser-chrome test. r=felipe,glandium

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

Comment 4

[Matthew N. [:MattN]]

Comment on attachment 8697332 [details]
MozReview Request: Bug 1169179 - Run mozscreenshots as a mochitest-browser-chrome test. r=felipe,glandium

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

Comment 5

[Mike Hommey [:glandium]]

Comment on attachment 8697332 [details]
MozReview Request: Bug 1169179 - Run mozscreenshots as a mochitest-browser-chrome test. r=felipe,glandium

https://reviewboard.mozilla.org/r/27559/#review25023

::: browser/tools/mozscreenshots/mozscreenshots/extension/Makefile.in:10
(Diff revision 3)
> +libs-preqs = \
> +  $(call mkdir_deps,$(TEST_EXTENSIONS_DIR)) \
> +  $(NULL)

You can replace this by GENERATED_DIRS = $(TEST_EXTENSIONS_DIR)

(see e.g. dom/workers/test/extensions/bootstrap/Makefile.in)

::: browser/tools/mozscreenshots/mozscreenshots/extension/Makefile.in:16
(Diff revision 3)
> +	$(NSINSTALL) -D $(DEPTH)/_tests/testing/mochitest/extensions/mozscreenshots

You should just add $(DEPTH)/_tests/testing/mochitest/extensions/mozscreenshots to GENERATED_DIRS

::: browser/tools/mozscreenshots/mozscreenshots/extension/Makefile.in:17
(Diff revision 3)
> +	cp -RL $(DEPTH)/testing/mozscreenshots/mozscreenshots $(DEPTH)/_tests/testing/mochitest/extensions

This seems like a convoluted way to put things in _tests/testing/mochitest/extensions. Why not just put them there in the first place, instead of "buffering" in testing/mozscreenshots/mozscreenshots?

Comment 6

[Matthew N. [:MattN]]

Attachment: MozReview Request: Bug 1169179 - Run mozscreenshots as a mochitest-browser-chrome test. r=felipe,glandium

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/27559/diff/3-4/

Comment 7

[Mike Hommey [:glandium]]

Comment on attachment 8698751 [details]
MozReview Request: Bug 1169179 - Run mozscreenshots as a mochitest-browser-chrome test. r=felipe,glandium

https://reviewboard.mozilla.org/r/27559/#review25155

::: browser/tools/mozscreenshots/mozscreenshots/extension/Makefile.in:7
(Diff revision 4)
> +GENERATED_DIRS += $(DEPTH)/_tests/testing/mochitest/extensions/mozscreenshots

Considering the content of this Makefile now, this line shouldn't be needed at all.

Comment 8

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 8697332 [details]
MozReview Request: Bug 1169179 - Run mozscreenshots as a mochitest-browser-chrome test. r=felipe,glandium

https://reviewboard.mozilla.org/r/27559/#review27917

::: .eslintignore:87
(Diff revision 4)
> +browser/tools/mozscreenshots/mozscreenshots/extension/bootstrap.js
> +browser/tools/mozscreenshots/mozscreenshots/extension/TestRunner.jsm

let's see if we can remove this

::: browser/tools/mozscreenshots/mozscreenshots/extension/TestRunner.jsm:45
(Diff revision 4)
> +        screenshotPath = FileUtils.getFile("TmpD", ["mozscreenshots"]).path + "\\";
> +        break;
> +      case "Darwin":
> +      case "Linux":
> +        screenshotPath = "/tmp/mozscreenshots/";

why not use TmpD on OSX/Linux too?

::: browser/tools/mozscreenshots/mozscreenshots/extension/bootstrap.js:1
(Diff revision 4)
> +#if 0
> +# This Source Code Form is subject to the terms of the Mozilla Public
> +# License, v. 2.0. If a copy of the MPL was not distributed with this file,
> +# You can obtain one at http://mozilla.org/MPL/2.0/.
> +#endif

maybe wrapping this in a comment block can keep this file lintable and make the pre-processor happy

Comment 9

[Matthew N. [:MattN]]

Comment on attachment 8697332 [details]
MozReview Request: Bug 1169179 - Run mozscreenshots as a mochitest-browser-chrome test. r=felipe,glandium

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/27559/diff/4-5/

Comment 10

[Matthew N. [:MattN]]

Comment on attachment 8698751 [details]
MozReview Request: Bug 1169179 - Run mozscreenshots as a mochitest-browser-chrome test. r=felipe,glandium

mozreview seems to be confused with the two attachments

Comment 11

[Pulsebot]

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

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/162b1fb7424a

Comment 13

[Mayur Patil]

[bugday-20160323]

Status: RESOLVED,FIXED -> VERIFIED

Comments:
STR: Not clear. Developer specific testing

Component: 
Name 			Firefox
Version 		46.0b4
Build ID 		20160322075646
Update Channel 	        beta
User Agent 		Mozilla/5.0 (Windows NT 6.1; WOW64; rv:46.0) Gecko/20100101 Firefox/46.0
OS                      Windows 7 SP1 x86_64

Actual Results: 
As expected

Expected Results: 
Please improve STR.