MozillaWiki

Auto-tools/Projects/Stockwell/disable-recommended: Difference between revisions

Page Discussion

Auto-tools/Projects/Stockwell/disable-recommended (view source)

Revision as of 09:11, 12 December 2019

8,577 bytes added ,  12 December 2019
m
updated the post disable steps and reviewers
No edit summary
m (updated the post disable steps and reviewers)
 
(43 intermediate revisions by 4 users not shown)
Line 1: Line 1:
= This page is under construction! =
= [stockwell disable-recommended] =
 
== [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 ===
== 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.
** Ignore "meta" bugs or any bugs not related to a particular test.
** Ignore "meta" bugs or any bugs not related to a particular test, some examples:
*** {{bug|1391545}}
*** {{bug|1391545}}
*** {{bug|1407383}}
*** {{bug|1407383}}
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 ===
== 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,
[[File:Searchfox_manifest.jpg|500px]]
* Run 'mach test-info --show-info <test-name>' in a mozilla-central check-out.
* Run 'mach test-info --show-info <test-name>' in a mozilla-central check-out.
<pre>
$ ./mach test-info --show-info browser/base/content/test/general/browser_ctrlTab.js
===== browser/base/content/test/general/browser_ctrlTab.js =====
Found browser\base\content\test\general\browser_ctrlTab.js in source control.
Build configuration changed. Regenerating backend.
browser\base\content\test\general\browser_ctrlTab.js found in manifest browser/base/content/test/general/browser.ini
  flavor: browser-chrome
</pre>


=== Determining which platforms are affected ===
In the above cases you would want to edit browser/base/content/test/general/browser.ini
<pre>
$ pwd
$ /home/mozilla/mozilla-central
$ nano browser/base/content/test/general/browser.ini
</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?


=== Updating a manifest to disable a test ===
There are many reasons why a test might only fail on certain configs:
* we don't run that test on a specific platform (for example devtools on android)
* the test is skipped already on a configuration or platform
 
We typically have 4 platforms:
* Linux (opt|debug|pgo|asan|ccov) - also bits (32|64)
* OSX - (opt|debug) - only run on 10.10
* Windows - (opt|debug|pgo|ccov) - also Windows 7|10
* Android - (opt|debug) - x86|arm
** no browser-chrome, devtools, web-platform-tests, and a few others
 
If the test is failing at least 5 times in the last 7 days on any given config, lets skip it.  We cannot skip on pgo specifically, so that is opt (!debug).  Often I try to make the skip syntax as simple as possible for simplicity sake and future editing.  If we have:
<pre>
[browser_sanity.js]
skip-if = (os == 'linux' && debug && bits == 64) || (os == 'linux' && !debug && bits == 64) || (os == 'osx' && !debug)
</pre>
 
I would prefer to see:
<pre>
[browser_sanity.js]
skip-if = (os == 'linux' && bits == 64) || (os == 'osx' && !debug)
</pre>
 
If the test already had a skip if:
<pre>
[browser_sanity.js]
skip-if = (os == 'win') # Bug 3141592 - timeout
</pre>
 
we would end up with:
<pre>
[browser_sanity.js]
skip-if = true # Bug 3141592, 271289
</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.
 
== Updating a manifest to disable a test ==
There are 3 different types of manifest files, each with a different format:
* "'''ini'''" manifests like mochitest.ini, chrome.ini, browser.ini, xpcshell.ini
* '''reftest''' or "'''list'''" manifests like reftest.list, crashtest.list
* '''web-platform''' manifests in testing/web-platform/meta
==== ini manifests ====
* Manifest example: https://searchfox.org/mozilla-central/source/browser/base/content/test/about/browser.ini
 
* In this type of manifest, add a "skip-if" statement '''under''' the test name -- on the '''next''' line.
 
[some_test.html]
skip-if = ...
 
* Some examples of "skip-if":
skip-if = true                            # skip this test everywhere / always (test never runs)
skip-if = os == "android"                  # skip on Android only (continues to run on Linux, Windows, etc.)
                                            # other os strings: "win", "linux", "mac"
skip-if = debug                            # skip on all debug builds
skip-if = !debug                          # skip on all non-debug builds (opt, pgo)
skip-if = os == "android" || os == "linux" # skip on Android and skip on Linux
skip-if = os == "android" && debug        # skip on Android/Debug only (continues to run on Android/Opt)
 
* For any test, there can only be one "skip-if" line. Often it is necessary to use || to expand the scope of a skip-if. For example, if a manifest already has:
 
[some_test.html]
skip-if = os == "android" || debug
 
but some_test.html is still failing on Windows/opt, it can be changed to:
 
[some_test.html]
skip-if = os == "android" || debug || os == "win"
 
* other keywords: '''os_version, bits, e10s, stylo, webrender, asan'''
* Windows os versions: https://msdn.microsoft.com/en-us/library/windows/desktop/ms724832(v=vs.85).aspx
 
==== reftest manifests ====
* Manifest example: https://searchfox.org/mozilla-central/source/dom/canvas/test/reftest/filters/reftest.list
* In this type of manifest, add a skip-if() statement on the '''same''' line as the test:
 
  skip-if(...) == some_test.html some_ref.html
 
* Some examples of "skip-if":
 
skip-if(gtkWidget)                  # skip on Linux
skip-if(cocoaWidget)                # skip on Mac
skip-if(winWidget)                  # skip on Windows
skip-if(Android)                    # skip on Android
skip-if(isDebugBuild)              # skip on all Debug builds
skip-if(!isDebugBuild)              # skip on all non-Debug builds
skip-if(Android&&isDebugBuild)      # skip on Android/Debug
 
* Other keywords: '''stylo, webrender, oscpu'''
* Reference: https://searchfox.org/mozilla-central/source/layout/tools/reftest/README.txt#42
 
==== web-platform test manifests ====
web-platform tests are found in the mozilla-central repo under testing/web-platform/tests. These tests have their own, unique manifest format. Also, not all tests are listed in a manifest: A manifest is only created when a test needs to be skipped or otherwise annotated.
 
Manifests for web-platform tests are found in testing/web-platform/meta. The /meta directory structure parallels the /tests structure. For example testing/web-platform/meta/2dcontext/building-paths has manifests related to the tests in testing/web-platform/tests/2dcontext/building-paths.
 
When disabling a web-platform test, check to see if there is an existing manifest in the /meta directory for that test: If there is, modify the existing manifest as required; if not, add a new file (remember to add the new file to your patch with 'hg add').
 
Here's an example of a patch disabling testing/web-platform/meta/content-security-policy/reporting/multiple-report-policies.html, on Linux and Windows 10/debug:
 
  diff --git a/testing/web-platform/meta/content-security-policy/reporting/multiple-report-policies.html.ini b/testing/web-platform/meta/content-security-policy/reporting/multiple-report-policies.html.ini
  new file mode 100644
  --- /dev/null
  +++ b/testing/web-platform/meta/content-security-policy/reporting/multiple-report-policies.html.ini
  @@ -0,0 +1,4 @@
  +[multiple-report-policies.html]
  +  disabled:
  +    if (os == "linux"): 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
 
== 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:


=== Requesting review ===
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 diff -c .                                                //see the changes you made to the files and self review
moz-phab                                                    //upload the changes to phabricator


=== After the test is disabled ===
NOTE:
* Check OrangeFactor to see if the patch was effective!
* 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.


= Notes from email =
NOTE: the self review is a great time to make sure:
Before disabling or when you don't disable:
* there are not extra spaces on a blank line or at the end of a line, if there are they will be highlighted in red
* you have to read the history and see if anyone is working on it or trying to figure out the bug
* you are only editing one line in a file, not multiple lines on accident
* review recent bug history
* a chance to review your syntax and any comments
** if there is a patch under review or comments indicate that a fix
is coming soon, consider postponing disabling
* look at the last 7 days in orange factor
** if no longer failing, mark [stockwell fixed]
** some examples:
** https://bugzilla.mozilla.org/show_bug.cgi?id=1420472
** https://bugzilla.mozilla.org/show_bug.cgi?id=1421775
** https://bugzilla.mozilla.org/show_bug.cgi?id=1423386 (fixed 2 days ago- recent comment and proved on graph)


* if it is a harness (i.e. not a clear test case in the title), then comment and stress the importance
There are additional, generic instructions for getting Mozilla reviews at:
** https://bugzilla.mozilla.org/show_bug.cgi?id=1391545
https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch
** https://bugzilla.mozilla.org/show_bug.cgi?id=1407383
**


if there are tests to disable we need to edit the manifests:
== After the test is disabled ==
* What type of test is this and where is the manifest? ./mach test-info
* Add leave-open to keyword and in Whiteboard change to [stockwell:disabled]
* Which platforms are affected? opt/debug/both, os, other
* "Watch" your patch landing to see if the patch was effective:
* One time mercurial setup:
** Are there any new failures on your push, or on the next few pushes, of the disabled test?
https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Source_Code/Mercurial
* Or, check OrangeFactor (maybe the next day) to see if any new failures are reported.
* Creating a patch and requesting review


We'll need separate guides to disabling syntax for:
== Special cases / getting help ==
* manifest parser
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.
** mochitest.ini, browser.ini, chrome.ini, xpcshell.ini, a11y.ini
* reftest manifests
** reftest.list, crashtests.list
** use fuzzing if possible
* web-platform tests
** testing/web-platform
* autophone (easily confused with above .ini/.list manifests)
Aciure
Confirmed users
2

edits

Retrieved from "https://wiki.allizom.org/Special:MobileDiff/1187052...1221233"