Split browser_inspector_menu.js into multiple tests

RESOLVED FIXED in Firefox 38

Status

DevTools
Inspector
RESOLVED FIXED
4 years ago
a month ago

People

(Reporter: Sami Jaktholm, Assigned: Sami Jaktholm)

Tracking

unspecified
Firefox 39
x86_64
Linux
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox38 fixed, firefox39 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

4 years ago
The inspector test browser_inspector_menu.js should be split up.

From bug 988314 by Patrick Brosset:
"I think we should have a test that only checks for existence and activate of menu items, and at least another one that checks that the items actually do what they say they do."
(Assignee)

Comment 1

3 years ago
Created attachment 8577709 [details] [diff] [review]
bug-1035140-inspector-menu-tests.patch

This patch splits the two browser_inspector-menu tests into four parts:
1) browser_inspector-menu-01-sensitivity.js combines all menu item sensitivity tests from the two tests into a single file.
2) browser_inspector-menu-02-copy-items.js tests the different type of copy actions
3) browser_inspector-menu-03-paste-items.js tests the functionality of different paste actions. This file is created by renaming the old browser_inspector-menu-02.js and removing sensitivity tests.
4) browser_inspector-menu-04-other.js tests the remaining items that don't fit into any specific category like copy or paste items. It has been created by renaming the old browser_inspector-menu-01.js and removing sensitivity tests.

Also, following changes was made compared to the combined functionality of the two old tests:
- Added a test for copy-image item. I think it's tested by markupview tests but it doesn't test the menuitem - it only tests the method that should do the heavy lifting.
- Added the 'Show DOM properties' item to the sensitivity tests
- Made the tests use the same document which is the old doc_inspector-menu-02.html + image for the copy image data test
- Fixed the 'TypeError: jsterm.focusInput is not a function' rejection to remove the whitelist from the test.
- Changed the root item delete item test to use "html" instead of inspector.walker.rootNode as the rootNode is not actually visible in the inspector and it was only spewing errors to the test log for missing editor.

Other than that, the new tests shouldn't differ from the old tests.

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c834259d920f
Assignee: nobody → sjakthol
Status: NEW → ASSIGNED
Attachment #8577709 - Flags: review?(pbrosset)
Comment on attachment 8577709 [details] [diff] [review]
bug-1035140-inspector-menu-tests.patch

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

Thanks for this test refactoring Sami. No comments about this change. Looks good.
Attachment #8577709 - Flags: review?(pbrosset) → review+
(Assignee)

Updated

3 years ago
Keywords: checkin-needed
https://hg.mozilla.org/integration/fx-team/rev/3871b59eaa49
Flags: in-testsuite+
Keywords: checkin-needed
Whiteboard: [fixed-in-fx-team]
I landed this on Aurora as well to ease any potential future backport pain during the esr38 lifecycle.
https://hg.mozilla.org/releases/mozilla-aurora/rev/8cb4760b0f54
status-firefox38: --- → fixed
https://hg.mozilla.org/mozilla-central/rev/3871b59eaa49
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox39: --- → fixed
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 39

Updated

a month ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.