Closed
Bug 1169179
(mozscreenshots)
Opened 10 years ago
Closed 9 years ago
Run mozscreenshots via mochitest-browser-chrome
Categories
(Testing :: mozscreenshots, enhancement)
Testing
mozscreenshots
Tracking
(firefox46 fixed)
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
firefox46 | --- | fixed |
People
(Reporter: MattN, Assigned: MattN)
References
(Depends on 4 open bugs, )
Details
Attachments
(1 file, 2 obsolete files)
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.
Assignee | ||
Comment 1•9 years ago
|
||
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
`
Assignee | ||
Updated•9 years ago
|
Attachment #8612148 -
Attachment is obsolete: true
Assignee | ||
Comment 2•9 years ago
|
||
Bug 1169179 - Run mozscreenshots as a mochitest-browser-chrome test. r=felipe,glandium
Attachment #8697332 -
Flags: review?(mh+mozilla)
Attachment #8697332 -
Flags: review?(felipc)
Assignee | ||
Comment 3•9 years ago
|
||
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/
Assignee | ||
Comment 4•9 years ago
|
||
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•9 years ago
|
||
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?
Attachment #8697332 -
Flags: review?(mh+mozilla)
Assignee | ||
Comment 6•9 years ago
|
||
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/27559/diff/3-4/
Attachment #8698751 -
Flags: review?(mh+mozilla)
Attachment #8698751 -
Flags: review?(felipc)
Comment 7•9 years ago
|
||
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.
Attachment #8698751 -
Flags: review?(mh+mozilla) → review+
Assignee | ||
Updated•9 years ago
|
Attachment #8697332 -
Attachment is obsolete: true
Attachment #8697332 -
Flags: review?(felipc)
Comment 8•9 years ago
|
||
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
Attachment #8697332 -
Flags: review+
Assignee | ||
Updated•9 years ago
|
Attachment #8698751 -
Flags: review?(felipc) → review+
Assignee | ||
Comment 9•9 years ago
|
||
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/
Attachment #8697332 -
Attachment is obsolete: false
Attachment #8697332 -
Flags: review?(mh+mozilla)
Assignee | ||
Comment 10•9 years ago
|
||
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
Attachment #8698751 -
Attachment is obsolete: true
Assignee | ||
Updated•9 years ago
|
Attachment #8697332 -
Flags: review?(mh+mozilla) → review+
Comment 12•9 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox46:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 46
Assignee | ||
Updated•9 years ago
|
Alias: mozscreenshots
Comment 13•9 years ago
|
||
[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.
Assignee | ||
Updated•7 years ago
|
Component: General → mozscreenshots
Product: Firefox → Testing
Target Milestone: Firefox 46 → ---
You need to log in
before you can comment on or make changes to this bug.
Description
•