Confirmed users
2
edits
Auto-tools/Projects/Stockwell/disable-recommended: Difference between revisions
Auto-tools/Projects/Stockwell/disable-recommended (view source)
Revision as of 09:11, 12 December 2019
, 12 December 2019updated the post disable steps and reviewers
m (updated the post disable steps and reviewers) |
|||
| (10 intermediate revisions by 4 users not shown) | |||
| Line 1: | Line 1: | ||
= [stockwell disable-recommended] = | |||
When an intermittent-failure bug has recorded 150 or more failures in the last 21 days, the Orange Factor robot will change the bug's stockwell whiteboard tag to "[stockwell disable-recommended]". These bugs should be reviewed at least twice per week (current schedule is Monday and Thursday). | When an intermittent-failure bug has recorded 150 or more failures in the last 21 days, the Orange Factor robot will change the bug's stockwell whiteboard tag to "[stockwell disable-recommended]". These bugs should be reviewed at least twice per week (current schedule is Monday and Thursday). | ||
This page describes how to review [stockwell disable-recommended] bugs. | This page describes how to review [stockwell disable-recommended] bugs. | ||
== Finding bugs to disable == | |||
* Use bugzilla to find all bugs with whiteboard tag "[stockwell disable-recommended]". | * Use bugzilla to find all bugs with whiteboard tag "[stockwell disable-recommended]". | ||
* For each disable-recommended bug, determine if the bug can be addressed by disabling tests. | * For each disable-recommended bug, determine if the bug can be addressed by disabling tests. | ||
| Line 23: | Line 21: | ||
*** If there is a patch under review or comments indicate that a fix is coming soon, consider postponing action on this bug. | *** If there is a patch under review or comments indicate that a fix is coming soon, consider postponing action on this bug. | ||
== Finding the manifest == | |||
For each test to be disabled, find the associated test manifest: | For each test to be disabled, find the associated test manifest: | ||
* Search for the test name in searchfox or dxr, or, | * Search for the test name in searchfox or dxr, or, | ||
| Line 45: | Line 43: | ||
</pre> | </pre> | ||
== Determining which platforms are affected == | |||
On OrangeFactor, check the detail view for the bug for the last 30 days. Look down the list of bugs for affected Platforms and Build Types. Does this test need to be disabled on all platforms, or only on some? For opt or debug builds, or both? | On OrangeFactor, check the detail view for the bug for the last 30 days. Look down the list of bugs for affected Platforms and Build Types. Does this test need to be disabled on all platforms, or only on some? For opt or debug builds, or both? | ||
| Line 83: | Line 81: | ||
</pre> | </pre> | ||
The reason for this is we would be skipping on linux/windows/osx, but not linux32 or android. Since this test doesn't run on Android, we would only be skipping on linux32- and a test that only runs on 1 platform doesn't provide a lot of value unless it is platform specific. | The reason for this is we would be skipping on linux/windows/osx, but not linux32 or android. Since this test doesn't run on Android, we would only be skipping on linux32 - and a test that only runs on 1 platform doesn't provide a lot of value unless it is platform specific. | ||
== Updating a manifest to disable a test == | |||
There are 3 different types of manifest files, each with a different format: | There are 3 different types of manifest files, each with a different format: | ||
* "'''ini'''" manifests like mochitest.ini, chrome.ini, browser.ini, xpcshell.ini | * "'''ini'''" manifests like mochitest.ini, chrome.ini, browser.ini, xpcshell.ini | ||
| Line 158: | Line 156: | ||
+ if debug and (os == "win") and (version == "10.0.15063"): https://bugzilla.mozilla.org/show_bug.cgi?id=1435526 | + if debug and (os == "win") and (version == "10.0.15063"): https://bugzilla.mozilla.org/show_bug.cgi?id=1435526 | ||
* If the test has expectations set, make sure to leave a blank line between the disabling conditions and the expectation. e.g.: https://hg.mozilla.org/integration/autoland/rev/b07579851b6f#l1.7 | |||
To actually disable a test, we need to update the manifest file in mozilla-central. Before doing that, create a | |||
== Making a patch == | |||
To actually disable a test, we need to update the manifest file in mozilla-central. Before doing that, create a patch using [https://moz-conduit.readthedocs.io/en/latest/phabricator-user.html?highlight=moz-phab#mozilla-phabricator-user-guide moz-phab] containing the change and upload the patch to bugzilla for review. | |||
For info about [https://github.com/mozilla-conduit/review/blob/master/README.md#installation installing] and [https://github.com/mozilla-conduit/review/blob/master/README.md#configuration configuring] moz-phab and implicitly arcanist see this [https://moz-conduit.readthedocs.io/en/latest/phabricator-user.html#setting-up-arcanist page] | |||
After moz-phab setup you need to follow these steps to upload your patch to Phabricator: | |||
nano/gedit file_path //edit the file you want to disable | |||
hg commit //add commit message | |||
<inside editor add comment |Bug 1234567 - disable <test> for frequent failures. r=#intermittent-reviewers| and save> | |||
hg diff -c . //see the changes you made to the files and self review | |||
moz-phab //will submit the changes in phabricator | |||
If you want to update an existing phabricator patch, you need to be the owner of the revision or to [https://moz-conduit.readthedocs.io/en/latest/phabricator-user.html?highlight=commandeer#other-revision-actions commandeer] it. Then do the following steps: | |||
moz-phab patch patch_id --apply-to . //applies the patch locally to the active repo (recommended mozilla-central) | |||
nano/gedit file_path //edit the file you need to change | |||
hg commit --amend //update or change the commit | |||
hg | hg diff -c . //see the changes you made to the files and self review | ||
hg | moz-phab //upload the changes to phabricator | ||
NOTE: | |||
* The comment for the commit needs to follow the general pattern above: "Bug xxx - <patch description>. r=yyy" | |||
* Phabricator automatically detects the reviewer from the commit message and asks for a review to the specified person. | |||
* In most cases, for disable patches use r=gbrown or r=jmaher, except if you are disabling for webrender only, then use r=kats. | |||
NOTE: the self review is a great time to make sure: | NOTE: the self review is a great time to make sure: | ||
| Line 184: | Line 187: | ||
* you are only editing one line in a file, not multiple lines on accident | * you are only editing one line in a file, not multiple lines on accident | ||
* a chance to review your syntax and any comments | * a chance to review your syntax and any comments | ||
There are additional, generic instructions for getting Mozilla reviews at: | There are additional, generic instructions for getting Mozilla reviews at: | ||
https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch | https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch | ||
== After the test is disabled == | |||
* Add leave-open to keyword and in Whiteboard change to [stockwell:disabled] | |||
* "Watch" your patch landing to see if the patch was effective: | * "Watch" your patch landing to see if the patch was effective: | ||
** Are there any new failures on your push, or on the next few pushes, of the disabled test? | ** Are there any new failures on your push, or on the next few pushes, of the disabled test? | ||
* Or, check OrangeFactor (maybe the next day) to see if any new failures are reported. | * Or, check OrangeFactor (maybe the next day) to see if any new failures are reported. | ||
= | == Special cases / getting help == | ||
There are lots of "special cases" -- different types of tests, platform variations, and who knows what! If something seems odd, if you need more information, we're here to help: Ping/email :gbrown / :jmaher. | |||