Alphabetize the toolkit/content/tests/widgets/chrome.ini manifest
Categories
(Toolkit :: UI Widgets, task)
Tracking
()
Tracking | Status | |
---|---|---|
firefox112 | --- | fixed |
People
(Reporter: hjones, Assigned: siya066btit21, Mentored)
Details
(Keywords: good-first-bug)
Attachments
(1 file, 1 obsolete file)
The test listed in toolkit/content/tests/widgets/chrome.ini
are not in alphabetical order, which violates the newly added test-manifest
lint rule. This was flagged when the manifest was changed in a recent patch: https://phabricator.services.mozilla.com/D171238#C6101706NL1
The correct order of tests in that file should be:
test_contextmenu_menugroup.xhtml
test_contextmenu_nested.xhtml
test_editor_currentURI.xhtml
test_image_recognition.html
test_image_recognition_unsupported.html
test_image_recognition_zh.html
test_label_checkbox.xhtml
test_menubar.xhtml
test_moz_button_group.html
test_moz_support_link.html
test_moz_toggle.html
test_panel_item_accesskey.html
test_panel_list_accessibility.html
test_panel_list_in_xul_panel.html
test_popupanchor.xhtml
test_popupreflows.xhtml
test_tree_column_reorder.xhtml
test_videocontrols_focus.html
test_videocontrols_onclickplay.html
To help Mozilla out with this bug, follow these steps:
- Comment here on the bug that you want to volunteer to help. This will tell others that you're working on the next steps.
- Download and build the Firefox source code
- If you have any problems, please ask on Element/Matrix in the
#introduction
channel. They're there to help you get started. - You can also read the Firefox Contributors' Quick Reference, which has answers to most development questions.
- If you have any problems, please ask on Element/Matrix in the
- Start working on this bug.
- Go to the test manifest file and re-order the files listed so that they are in alphabetical order. This document has detailed information on our test manifests, including an explanation of how the conditional syntax works.
- If you have any problems with this bug, please comment on this bug and set the needinfo flag for me. Also, you can find me and my teammates on the
#reusable-components
channel on Element/Matrix most hours of most days.
- Build your change with
mach build
and then run the tests in that file via the command./mach test toolkit/content/tests/widgets/chrome.ini
. More information on running tests can be found here. Also check that the manifest now passes outtest-manifest
lint rule by running./mach lint
- Submit the patch (including an automated test, if applicable) for review. Mark me as a reviewer so I'll get an email to come look at your code.
- Getting your code reviewed
- This is when the bug will be assigned to you.
- After a series of reviews and changes to your patch, I'll mark it for checkin or push it to autoland. Your code will soon be shipping to Firefox users worldwide!
Hey, can I please be assigned to this issue? I already have firefox up and running on my local machine and would love to work on this. Thank you.
Hi Hanna, I am looking into this bug and interested in working on the next steps.
Reporter | ||
Comment 3•1 year ago
|
||
Hey, thanks for your interest! I'm assigning this to Siya for now since they asked to be assigned first. In the event that they are not able to submit a patch we may open this back up again.
Hey Hanna, so according to the alphabetical order of lists that you have provided, [test_moz_support_link.html] shouldn't be a part of the manifest but since it is present in the "toolkit/content/tests/widgets/chrome.ini" file, should I just remove it? Also, [test_moz_label.html] is not in the chrome.ini manifest, so I am assuming I am supposed to add it, however I don't see a test_moz_label.html file present in the "toolkit/content/tests/widgets" folder either. How can I proceed?
Reporter | ||
Comment 5•1 year ago
|
||
Hey Siya - good catch! There was an error in the manifest list I had provided. I just updated it so it should be correct now. test_moz_support_link.html
should be in the manifest, but test_moz_label.html
should not. I copied that list over from a work in progress patch that is adding test_moz_label
, sorry for the confusion. Let me know if you have any other questions.
Hello, just ran the tests and it shows an unexpected result saying FAIL Test Timed out, -
at toolkit/content/tests/widgets/test_popupanchor.xhtml
. Should I just proceed with further steps? I think a test is failing, but I'm not sure. Please help.
Reporter | ||
Comment 7•1 year ago
|
||
Fist I would check to make sure that as you've alphabetized the manifest you've moved all the skip-if
lines correctly - those lines apply to the line above them, so if they get moved tests could be running on a platform where they're always going to fail.
It also might be worth putting up a patch at this point so that I can take a look and see if I'm encountering the same test failure. If the only thing that changed is the order the tests then it's unlikely that the failure you're seeing is due to your changes, so I wouldn't worry too much. This document has good instructions on how to submit your code for review.
Reporter | ||
Comment 9•1 year ago
|
||
Tests pass for me locally and your changes look good! I just approved the patch, pending one small whitespace change. Once you make the change, you'll need to submit the patch again. You can find instructions on how to do that here.
I'll also encourage you to try to find another bug to pick up from this list since you have the project working locally and you're off to a good start. Nice job!
Assignee | ||
Comment 10•1 year ago
|
||
Depends on D172011
Comment 11•1 year ago
|
||
Pushed by hjones@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/8cce4dd68c4a Alphabetized the toolkit/content/tests/widgets/chrome.ini manifest. r=hjones
Updated•1 year ago
|
Comment 12•1 year ago
|
||
bugherder |
Description
•