939271 - Move various mochitests to manifests

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Comment 1

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Part a: hal

The test is listed both in hal/Makefile.in and hal/tests/browser.ini, so it's being run twice.

Comment 2

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Part b: caps

Comment 3

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Part c: browser/devtools/styleinspector/test

Comment 4

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Part d: toolkit/content/tests/widgets

Comment 5

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Part e: toolkit/components/social/test/browser

Comment 6

[Bobby Holley (:bholley)]

Comment on attachment 833130 [details] [diff] [review]
Part b: caps

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

I already did this in bug 939166.

Comment 7

[Michael Shal [:mshal]]

Comment on attachment 833131 [details] [diff] [review]
Part c: browser/devtools/styleinspector/test

>diff --git a/browser/devtools/styleinspector/test/Makefile.in b/browser/devtools/styleinspector/test/Makefile.in
>-MOCHITEST_BUG_677940_DATA_FILES = \
>+mochitest_bug_677940_data_FILES = \
>   browser_styleinspector_bug_677930_urls_clickable/browser_styleinspector_bug_677930_urls_clickable.css \
>   $(NULL)
>-MOCHITEST_BUG_677940_DATA_DEST = $(MOCHITEST_BROWSER_DEST)/browser_styleinspector_bug_677930_urls_clickable
>-INSTALL_TARGETS += MOCHITEST_BUG_677940_DATA
>+mochitest_bug_677940_data_DEST = $(mochitest_browser_dest)/browser_styleinspector_bug_677930_urls_clickable
>+INSTALL_TARGETS += mochitest_bug_677940_data

I think all '677940' numbers should be '677930'

Comment 8

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Part f: js/xpconnect

Comment 9

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Part g: gfx/tests

Comment 10

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Part h: parser

Comment 11

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Part i: crashreporter

Comment 12

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Part j: widget

Comment 13

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Part k: browser/devtools/commandline

Comment 14

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Part l: browser/devtools/debugger

Comment 15

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Part m: accessible/tests/mochitest

Comment 16

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

(In reply to Bobby Holley (:bholley) from comment #6)
> Comment on attachment 833130 [details] [diff] [review]
> Part b: caps
> 
> Review of attachment 833130 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I already did this in bug 939166.

Bug 840488, that is.

Comment 17

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Part m: accessible/tests/mochitest

Comment 18

[Henri Sivonen (:hsivonen)]

Comment on attachment 833432 [details] [diff] [review]
Part h: parser

rs=hsivonen.

Comment 19

[Trevor Saunders (:tbsaunde)]

Comment on attachment 833455 [details] [diff] [review]
Part m: accessible/tests/mochitest

>diff --git a/accessible/tests/mochitest/tree/a11y.ini b/accessible/tests/mochitest/tree/a11y.ini
>new file mode 100644
>--- /dev/null
>+++ b/accessible/tests/mochitest/tree/a11y.ini

I'm not sure what the point would be but should these files need license notices?

(they use rather odd format too)

>@@ -0,0 +1,40 @@
>+[DEFAULT]
>+support-files =
>+  dockids.html
>+  wnd.xul
>+
>+[test_applicationacc.xul]
>+skip-if = true # Bug 561508
>+[test_aria_globals.html]
>+[test_aria_grid.html]
>+[test_aria_imgmap.html]
>+[test_aria_list.html]
>+[test_aria_menu.html]
>+[test_aria_presentation.html]
>+[test_brokencontext.html]
>+[test_button.xul]
>+[test_canvas.html]
>+[test_combobox.xul]
>+[test_cssoverflow.html]
>+[test_dochierarchy.html]
>+[test_dockids.html]
>+[test_filectrl.html]
>+[test_formctrl.html]
>+[test_formctrl.xul]
>+[test_gencontent.html]
>+[test_groupbox.xul]
>+[test_iframe.html]
>+[test_img.html]
>+[test_invalid_img.xhtml]
>+[test_invalidationlist.html]
>+[test_list.html]
>+[test_map.html]
>+[test_media.html]
>+[test_select.html]
>+[test_tabbox.xul]
>+[test_tabbrowser.xul]
>+[test_table.html]
>+[test_tree.xul]
>+[test_txtcntr.html]
>+[test_txtctrl.html]
>+[test_txtctrl.xul]
>diff --git a/accessible/tests/mochitest/tree/moz.build b/accessible/tests/mochitest/tree/moz.build
>deleted file mode 100644
>--- a/accessible/tests/mochitest/tree/moz.build
>+++ /dev/null
>@@ -1,6 +0,0 @@
>-# -*- Mode: python; c-basic-offset: 4; indent-tabs-mode: nil; tab-width: 40 -*-
>-# vim: set filetype=python:
>-# 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/.
>-

Comment 20

[Michael Shal [:mshal]]

Comment on attachment 833445 [details] [diff] [review]
Part l: browser/devtools/debugger

>diff --git a/browser/devtools/debugger/test/browser.ini b/browser/devtools/debugger/test/browser.ini
>+[browser_dbg_break-on-dom-event.js]
>+skip-if = os == "mac" # Bug 895426
>+[browser_dbg_event-listeners.js]
>+skip-if = os == "mac"

Maybe add '# Bug 895426' to the last line as well? It's not so obvious that it applies to both.

Comment 21

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

(In reply to Michael Shal [:mshal] from comment #20)
> Comment on attachment 833445 [details] [diff] [review]
> Part l: browser/devtools/debugger
> 
> >diff --git a/browser/devtools/debugger/test/browser.ini b/browser/devtools/debugger/test/browser.ini
> >+[browser_dbg_break-on-dom-event.js]
> >+skip-if = os == "mac" # Bug 895426
> >+[browser_dbg_event-listeners.js]
> >+skip-if = os == "mac"
> 
> Maybe add '# Bug 895426' to the last line as well? It's not so obvious that
> it applies to both.

According to the comment in the makefile, it doesn't apply to both.

Comment 22

[Michael Shal [:mshal]]

(In reply to :Ms2ger from comment #21)
> (In reply to Michael Shal [:mshal] from comment #20)
> > Maybe add '# Bug 895426' to the last line as well? It's not so obvious that
> > it applies to both.
> 
> According to the comment in the makefile, it doesn't apply to both.

I was looking at the title of bug 895426, which mentions both tests. Though maybe not because of https://bugzilla.mozilla.org/show_bug.cgi?id=895426#c233

Comment 23

[Mounir Lamouri (:mounir)]

Comment on attachment 833129 [details] [diff] [review]
Part a: hal

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

I guess browser.ini is a new thing. Never heard about it before ;)

rs=me

Comment 24

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

https://hg.mozilla.org/mozilla-central/rev/c3dd078ab5ec
https://hg.mozilla.org/mozilla-central/rev/146bebf7e8c1
https://hg.mozilla.org/mozilla-central/rev/aaa2f184e278
https://hg.mozilla.org/mozilla-central/rev/636c4f4e7466
https://hg.mozilla.org/mozilla-central/rev/80f4e0b7574c
https://hg.mozilla.org/mozilla-central/rev/814701cdd1e4
https://hg.mozilla.org/mozilla-central/rev/c398e15e588a
https://hg.mozilla.org/mozilla-central/rev/a49a8eb4423f
https://hg.mozilla.org/mozilla-central/rev/60aff5910956
https://hg.mozilla.org/mozilla-central/rev/1ec55de09728
https://hg.mozilla.org/mozilla-central/rev/400b3d2e4584
https://hg.mozilla.org/mozilla-central/rev/89f6122e88fb