Drop test annotations and checks for Windows XP, Vista OS X < 10.9 and Android Gingerbread (2.3)

RESOLVED FIXED

Status

()

defect
RESOLVED FIXED
2 years ago
8 months ago

People

(Reporter: aryx, Assigned: aryx)

Tracking

(Blocks 1 bug)

unspecified
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox65 fixed)

Details

Attachments

(8 attachments, 11 obsolete attachments)

46 bytes, text/x-phabricator-request
RyanVM
: review+
Details | Review
46 bytes, text/x-phabricator-request
RyanVM
: review+
Details | Review
46 bytes, text/x-phabricator-request
bzbarsky
: review+
Details | Review
46 bytes, text/x-phabricator-request
RyanVM
: review+
Details | Review
46 bytes, text/x-phabricator-request
RyanVM
: review+
Details | Review
46 bytes, text/x-phabricator-request
RyanVM
: review+
Details | Review
46 bytes, text/x-phabricator-request
Details | Review
46 bytes, text/x-phabricator-request
RyanVM
: review+
Details | Review
Thanks to dholbert for putting the information together in bug 1334898:

OS X pre-10.9 unsupport was announced here (effective version 49, bug 1255589):
 https://blog.mozilla.org/futurereleases/2016/04/29/update-on-firefox-support-for-os-x/

WinXP/Vista unsupport was announced here (bug 1130266):
 https://blog.mozilla.org/futurereleases/2016/12/23/firefox-support-for-xp-and-vista/
I believe this newsgroup thread indicates that ~52 is the last supported version:

Last version with Android 2.3 (Gingerbread) support was Firefox for Android 47: bug 1220184

So let's ger rid of the test annotations.
Comment hidden (mozreview-request)

Comment 13

2 years ago
mozreview-review
Comment on attachment 8833623 [details]
Bug 1336712 - Drop test annotations and checks for Windows XP, Vista OS X < 10.9 and Android Gingerbread (2.3): accessibility.

https://reviewboard.mozilla.org/r/109824/#review110874

::: accessible/tests/mochitest/jsat/a11y.ini:15
(Diff revision 1)
>    !/accessible/tests/mochitest/moz.png
> -skip-if = (os == 'win' && (os_version == '5.1' || os_version == '5.2'))
>  
>  [test_alive.html]
>  [test_content_integration.html]
>  skip-if = buildapp == 'mulet'

Not directly related to your patch, but I thought we'd already removed all the Mulet references from the manifests. Guess not.
Attachment #8833623 - Flags: review?(ryanvm) → review+

Comment 14

2 years ago
mozreview-review
Comment on attachment 8833624 [details]
Bug 1336712 - Drop test annotations and checks for Windows XP, Vista OS X < 10.9 and Android Gingerbread (2.3): browser.

https://reviewboard.mozilla.org/r/109826/#review110876
Attachment #8833624 - Flags: review?(ryanvm) → review+

Comment 15

2 years ago
mozreview-review
Comment on attachment 8833625 [details]
Bug 1336712 - Drop test annotations and checks for Windows XP, Vista OS X < 10.9 and Android Gingerbread (2.3): devtools.

https://reviewboard.mozilla.org/r/109828/#review110878

::: devtools/shared/system.js:285
(Diff revision 1)
>  
>  /**
>   * Function for fetching OS CPU and returning
>   * an enum for Telemetry.
>   */
>  function getOSCPU() {

Might want to get someone from the devtools team to look at this change and decide if it's worth updating the return values from this function.
Attachment #8833625 - Flags: review?(ryanvm) → review+

Comment 16

2 years ago
mozreview-review
Comment on attachment 8833626 [details]
Bug 1336712 - Drop test annotations and checks for Windows XP, Vista OS X < 10.9 and Android Gingerbread (2.3): dom.

https://reviewboard.mozilla.org/r/109830/#review110880

Looks good in general. Need to get some sanity checks from people more familiar with some of these tests, though.

::: dom/canvas/test/test_canvas.html:37
(Diff revision 1)
> -}
> -
> -
>  function IsAzureSkia() {
>    var enabled = false;
>    

Please remove this extra whitespace while you're here.

::: dom/canvas/test/webgl-conf/generated-mochitest.ini:7836
(Diff revision 1)
>  [generated/test_conformance__glsl__misc__glsl-vertex-branch.html]
>  [generated/test_conformance__glsl__misc__large-loop-compile.html]
>  [generated/test_conformance__glsl__misc__non-ascii-comments.vert.html]
>  [generated/test_conformance__glsl__misc__non-ascii.vert.html]
>  [generated/test_conformance__glsl__misc__re-compile-re-link.html]
>  fail-if = (os == 'android' && android_version == '10')

This can go, right?

::: dom/canvas/test/webgl-conf/mochitest-errata.ini:122
(Diff revision 1)
>  
>  [generated/test_conformance__misc__type-conversion-test.html]
>  fail-if = (os == 'linux')
>  # Resets device on Android 2.3.
>  # Crashes on B2G ICS Emulator, desktop Linux, and Mulet Linux x64.
>  skip-if = (os == 'android') || (os == 'b2g') || (os == 'linux')

B2G, eh?

::: dom/media/mediasource/test/mochitest.ini:47
(Diff revision 1)
>    bipbop/bipbop_480_624kbps-videoinit.mp4 bipbop/bipbop_480_624kbps-videoinit.mp4^headers^
>    bipbop/bipbop_480_624kbps-video1.m4s bipbop/bipbop_480_624kbps-video1.m4s^headers^
>    bipbop/bipbop_480_624kbps-video2.m4s bipbop/bipbop_480_624kbps-video2.m4s^headers^
>  
>  [test_AudioChange_mp4.html]
> -skip-if = ((os == "win" && os_version == "5.1") || (toolkit == 'android')) # Not supported on xp and android 2.3
> +skip-if = toolkit == 'android' # Not supported on android 2.3

Looks like the comment could use updating on this one and the ones below.

::: dom/media/mediasource/test/mochitest.ini:68
(Diff revision 1)
>  [test_EndedEvent.html]
>  [test_EndOfStream.html]
>  [test_EndOfStream_mp4.html]
> -skip-if = ((os == "win" && os_version == "5.1") || (toolkit == 'android')) # Not supported on xp and android 2.3
> +skip-if = toolkit == 'android' # Not supported on android 2.3
>  [test_Eviction_mp4.html]
>  skip-if = (os == "win" && os_version == "5.1") # Not supported on xp.

missed one :)

::: dom/media/tests/mochitest/pc.js:1597
(Diff revision 1)
>      for (let [key, res] of stats) {
>        // validate stats
>        ok(res.id == key, "Coherent stats id");
>        var nowish = Date.now() + 1000;        // TODO: clock drift observed
>        var minimum = this.whenCreated - 1000; // on Windows XP (Bug 979649)
> -      if (isWinXP) {
> +      if (false) { // Bug 1325430 - timestamps aren't working properly in update 49

I'm thinking the WebRTC folks might want to clean this up a bit. Probably worth pinging whoever touched this code last.

::: dom/performance/tests/test_worker_performance_now.js:30
(Diff revision 1)
>  var n = self.performance.now(), d = Date.now();
>  ok(n >= 0, "The value of now() should be equal to or greater than 0.");
>  ok(self.performance.now() >= n, "The value of now() should monotonically increase.");
>  
>  // The spec says performance.now() should have micro-second resolution, but allows 1ms if the platform doesn't support it.
> -// Our implementation does provide micro-second resolution, except for windows XP combined with some HW properties
> +// Our implementation does provide micro-second resolution.

Please have someone more familiar with this test look it over as well.

::: dom/tests/mochitest/general/test_performance_now.html:20
(Diff revision 1)
>      ok(window.performance.now() >= n, "The value of now() should monotonically increase.");
>      SimpleTest.waitForExplicitFinish();
>      SimpleTest.requestFlakyTimeout("untriaged");
>  
>      // The spec says performance.now() should have micro-second resolution, but allows 1ms if the platform doesn't support it.
> -    // Our implementation does provide micro-second resolution, except for windows XP combined with some HW properties
> +    // Our implementation does provide micro-second resolution.

Also this one.
Attachment #8833626 - Flags: review?(ryanvm) → review+

Comment 17

2 years ago
mozreview-review
Comment on attachment 8833627 [details]
Bug 1336712 - Drop test annotations and checks for Windows XP, Vista OS X < 10.9 and Android Gingerbread (2.3): gfx.

https://reviewboard.mozilla.org/r/109832/#review110882
Attachment #8833627 - Flags: review?(ryanvm) → review+

Comment 18

2 years ago
mozreview-review
Comment on attachment 8833628 [details]
Bug 1336712 - Drop test annotations and checks for Windows XP, Vista OS X < 10.9 and Android Gingerbread (2.3): memory

https://reviewboard.mozilla.org/r/109834/#review110884
Attachment #8833628 - Flags: review?(ryanvm) → review+

Comment 19

2 years ago
mozreview-review
Comment on attachment 8833629 [details]
Bug 1336712 - Drop test annotations and checks for Windows XP, Vista OS X < 10.9 and Android Gingerbread (2.3): netwerk

https://reviewboard.mozilla.org/r/109836/#review110886
Attachment #8833629 - Flags: review?(ryanvm) → review+

Comment 20

2 years ago
mozreview-review
Comment on attachment 8833630 [details]
Bug 1336712 - Drop test annotations and checks for Windows XP, Vista OS X < 10.9 and Android Gingerbread (2.3): testing/firefox-ui

https://reviewboard.mozilla.org/r/109838/#review110888
Attachment #8833630 - Flags: review?(ryanvm) → review+

Comment 21

2 years ago
mozreview-review
Comment on attachment 8833631 [details]
Bug 1336712 - Drop test annotations and checks for Windows XP, Vista OS X < 10.9 and Android Gingerbread (2.3): testing/talos

https://reviewboard.mozilla.org/r/109840/#review110890
Attachment #8833631 - Flags: review?(ryanvm) → review+

Comment 22

2 years ago
mozreview-review
Comment on attachment 8833632 [details]
Bug 1336712 - Drop test annotations and checks for Windows XP, Vista OS X < 10.9 and Android Gingerbread (2.3): toolkit

https://reviewboard.mozilla.org/r/109842/#review110892
Attachment #8833632 - Flags: review?(ryanvm) → review+

Comment 23

2 years ago
mozreview-review
Comment on attachment 8833633 [details]
Bug 1336712 - Drop test annotations and checks for Windows XP, Vista OS X < 10.9 and Android Gingerbread (2.3): widget

https://reviewboard.mozilla.org/r/109844/#review110894
Attachment #8833633 - Flags: review?(ryanvm) → review+
Beautiful :)
Assignee

Comment 25

2 years ago
mozreview-review-reply
Comment on attachment 8833623 [details]
Bug 1336712 - Drop test annotations and checks for Windows XP, Vista OS X < 10.9 and Android Gingerbread (2.3): accessibility.

https://reviewboard.mozilla.org/r/109824/#review110874

> Not directly related to your patch, but I thought we'd already removed all the Mulet references from the manifests. Guess not.

These mulet occurrences are still here because I used a filename whitelist when I searched for these. Filed https://bugzilla.mozilla.org/show_bug.cgi?id=1336917
Comment on attachment 8833626 [details]
Bug 1336712 - Drop test annotations and checks for Windows XP, Vista OS X < 10.9 and Android Gingerbread (2.3): dom.

bz: Please check the cleanup in the two performance files at the end of the patch.

jesup: RyanVM asks if you want to do some cleanup in pc.js now that there is only an |if (false)| left at one position in the code.
Attachment #8833626 - Flags: review?(rjesup)
Attachment #8833626 - Flags: review?(bzbarsky)

Comment 27

2 years ago
mozreview-review
Comment on attachment 8833626 [details]
Bug 1336712 - Drop test annotations and checks for Windows XP, Vista OS X < 10.9 and Android Gingerbread (2.3): dom.

https://reviewboard.mozilla.org/r/109830/#review111078

All the comments on test_performance_now apply to the worker version of the test too.

::: dom/tests/mochitest/general/test_performance_now.html:19
(Diff revision 1)
>      ok(n >= 0, "The value of now() should be equal to or greater than 0.");
>      ok(window.performance.now() >= n, "The value of now() should monotonically increase.");
>      SimpleTest.waitForExplicitFinish();
>      SimpleTest.requestFlakyTimeout("untriaged");
>  
>      // The spec says performance.now() should have micro-second resolution, but allows 1ms if the platform doesn't support it.

Not anymore it doesn't.  See https://w3c.github.io/hr-time/#idl-def-domhighrestimestamp -- it's not allowed to be accurate to less than 5us.

::: dom/tests/mochitest/general/test_performance_now.html:20
(Diff revision 1)
>      ok(window.performance.now() >= n, "The value of now() should monotonically increase.");
>      SimpleTest.waitForExplicitFinish();
>      SimpleTest.requestFlakyTimeout("untriaged");
>  
>      // The spec says performance.now() should have micro-second resolution, but allows 1ms if the platform doesn't support it.
> -    // Our implementation does provide micro-second resolution, except for windows XP combined with some HW properties
> +    // Our implementation does provide micro-second resolution.

It doesn't.  It's clamped to 5us.  Please fix the comment.

::: dom/tests/mochitest/general/test_performance_now.html:44
(Diff revision 1)
>  
> -      // Loose spec: 1ms resolution, or 15ms resolution for the XP-low-res case.
> -      // We shouldn't test that dt is actually within 2/25ms since the iterations break if it isn't, and timeout could be late.
> -      ok(n2 > n, "Loose - the value of now() should increase within 2ms (or 25ms if low-res counter) (delta now(): " + (n2 - n) + " ms).");
> -
> -      // Strict spec: if it's not the XP-low-res case, while the spec allows 1ms resolution, it prefers microseconds, which we provide.
> +      // Loose spec: 1ms resolution.
> +      // We shouldn't test that dt is actually within 2ms since the iterations break if it isn't, and timeout could be late.
> +      ok(n2 > n, "Loose - the value of now() should increase within 2ms (delta now(): " + (n2 - n) + " ms).");
> +
> +      // Strict spec: while the spec allows 1ms resolution, it prefers microseconds, which we provide.

Again, fix the comment, please.
Attachment #8833626 - Flags: review?(bzbarsky) → review+

Comment 28

2 years ago
mozreview-review
Comment on attachment 8833626 [details]
Bug 1336712 - Drop test annotations and checks for Windows XP, Vista OS X < 10.9 and Android Gingerbread (2.3): dom.

https://reviewboard.mozilla.org/r/109830/#review111256

::: dom/media/tests/mochitest/pc.js:1597
(Diff revision 1)
>      for (let [key, res] of stats) {
>        // validate stats
>        ok(res.id == key, "Coherent stats id");
>        var nowish = Date.now() + 1000;        // TODO: clock drift observed
>        var minimum = this.whenCreated - 1000; // on Windows XP (Bug 979649)
> -      if (isWinXP) {
> +      if (false) { // Bug 1325430 - timestamps aren't working properly in update 49

Yes, please do get rid of the "if (false)" bit
Attachment #8833626 - Flags: review?(rjesup) → review+
Assignee

Comment 29

2 years ago
mozreview-review-reply
Comment on attachment 8833626 [details]
Bug 1336712 - Drop test annotations and checks for Windows XP, Vista OS X < 10.9 and Android Gingerbread (2.3): dom.

https://reviewboard.mozilla.org/r/109830/#review110880

> B2G, eh?

Bug 1336917
(In reply to Randell Jesup [:jesup] from comment #28)
> Comment on attachment 8833626 [details]
> Bug 1336712 - Drop test annotations and checks for Windows XP, Vista OS X <
> 10.9 and Android Gingerbread (2.3): dom.
> 
> https://reviewboard.mozilla.org/r/109830/#review111256
> 
> ::: dom/media/tests/mochitest/pc.js:1597
> (Diff revision 1)
> >      for (let [key, res] of stats) {
> >        // validate stats
> >        ok(res.id == key, "Coherent stats id");
> >        var nowish = Date.now() + 1000;        // TODO: clock drift observed
> >        var minimum = this.whenCreated - 1000; // on Windows XP (Bug 979649)
> > -      if (isWinXP) {
> > +      if (false) { // Bug 1325430 - timestamps aren't working properly in update 49
> 
> Yes, please do get rid of the "if (false)" bit
Please confirm to me (as a non-native speaker) that the |if (false)| with the |if (!twoMachines)| which is currently commented out and that was done for webrtc.org v49 in https://hg.mozilla.org/mozilla-central/rev/126348e718d0 and bug 1325430 still open.
Flags: needinfo?(rjesup)

Comment 31

2 years ago
mozreview-review
Comment on attachment 8833625 [details]
Bug 1336712 - Drop test annotations and checks for Windows XP, Vista OS X < 10.9 and Android Gingerbread (2.3): devtools.

https://reviewboard.mozilla.org/r/109828/#review113362

::: devtools/shared/system.js
(Diff revision 1)
>  /**
>   * Function for fetching OS CPU and returning
>   * an enum for Telemetry.
>   */
>  function getOSCPU() {
> -  if (oscpu.includes("NT 5.1") || oscpu.includes("NT 5.2")) {

Hmm, I'd say leave this block alone...  It's easier to follow what's happening here if we keep it as is.  We don't want to change the values since they get recorded in telemetry, and starting from 2 (by removing these lines) would seem odd.
Attachment #8833625 - Flags: review?(jryans) → review+
Looks like this has stalled for nearly a year. Any plans to revive this?
Flags: needinfo?(aryx.bugmail)
I have rebased Aryx's patches onto m-c. Since MozReview has since been retired, I had to pull the patches from the https://hg.mozilla.org/mozreview/gecko backup repo. I will likely fork the Android test annotation removal into a new bug.

Some XP test annotations have already been removed in bug 1335260.
Depends on: 1335260
Flags: needinfo?(aryx.bugmail)
See Also: → 979649
Here is a green Try run with the rebased patches. There are some orange test timeouts, but none of them look related to the removal of these test annotations and checks for old platforms:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f725cb3de39bf5c6ac6cf80e6a39f9768b8fa313

I'm clearing needinfo?jesup because I moved that patch (with r?jesup) to RTCP timestamp bug 979649.
Flags: needinfo?(rjesup)
Comment on attachment 9011306 [details]
Bug 1336712 - a11y: Drop test check for OS X 10.6. r?RyanVM

Ryan VanderMeulen [:RyanVM] has approved the revision.
Attachment #9011306 - Flags: review+
Comment on attachment 9011307 [details]
Bug 1336712 - dom: Drop test checks for OS X 10.5 and Windows XP. r?RyanVM

Ryan VanderMeulen [:RyanVM] has approved the revision.
Attachment #9011307 - Flags: review+
Firefox no longer supports Windows XP, so these test checks that allow for timeouts with 25 ms resolution can be removed. Also, rewrite some test logic and comments to make the test's intention clearer.

The 'getOSCPU' message handler can be removed from test_worker_performance_now.html because test_worker_performance_now.js no longer needs to check for Windows XP.

Stop setting the pref "privacy.reduceTimerPrecision" = false in test_performance_now.html. That pref removes performance.now()'s 1 ms resolution limit so the performance timer will run at full speed. By leaving the pref's default value, the test can assert that performance.now() is actually honoring the 1 ms limit.

I didn't remove "privacy.reduceTimerPrecision" = false for the worker test. The worker tests run an accelerated setTimeout() clock, so setTimeout(1) can time out in less than 1 ms. Leaving the pref "privacy.reduceTimerPrecision" = true causes hundreds of worker tests to run more slowly (in real time), which would increase test automation time.

Depends on D6578
Comment on attachment 9011315 [details]
Bug 1336712 - dom/canvas: Remove test checks for Android ICS (API Level 10). r?RyanVM

Ryan VanderMeulen [:RyanVM] has approved the revision.
Attachment #9011315 - Flags: review+
Comment on attachment 9011316 [details]
Bug 1336712 - dom/media: Remove test checks for Android GB (API Level 15). r?RyanVM

Ryan VanderMeulen [:RyanVM] has approved the revision.
Attachment #9011316 - Flags: review+
Attachment #8833633 - Attachment is obsolete: true
Comment on attachment 9011319 [details]
Bug 1336712 - widget: Drop test check for OS X 10.6. r?RyanVM

Ryan VanderMeulen [:RyanVM] has approved the revision.
Attachment #9011319 - Flags: review+
Attachment #8833623 - Attachment is obsolete: true
Attachment #8833624 - Attachment is obsolete: true
Attachment #8833625 - Attachment is obsolete: true
Attachment #8833626 - Attachment is obsolete: true
Attachment #8833627 - Attachment is obsolete: true
Attachment #8833628 - Attachment is obsolete: true
Attachment #8833629 - Attachment is obsolete: true
Attachment #8833630 - Attachment is obsolete: true
Attachment #8833631 - Attachment is obsolete: true
Attachment #8833632 - Attachment is obsolete: true
Comment on attachment 9011317 [details]
Bug 1336712 - netwerk: Drop test checks for Windows XP. r?RyanVM

Ryan VanderMeulen [:RyanVM] has approved the revision.
Attachment #9011317 - Flags: review+

Comment 49

9 months ago
Pushed by cpeterson@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/eace0121d8f7
a11y: Drop test check for OS X 10.6. r=RyanVM
https://hg.mozilla.org/integration/mozilla-inbound/rev/0cf26a604c28
dom: Drop test checks for OS X 10.5 and Windows XP. r=RyanVM
https://hg.mozilla.org/integration/mozilla-inbound/rev/cd8bd5ed0481
dom/canvas: Remove test checks for Android ICS (API Level 10). r=RyanVM
https://hg.mozilla.org/integration/mozilla-inbound/rev/406a83114d6c
dom/media: Remove test checks for Android GB (API Level 15). r=RyanVM
https://hg.mozilla.org/integration/mozilla-inbound/rev/3e815f61a1fc
netwerk: Drop test checks for Windows XP. r=RyanVM
https://hg.mozilla.org/integration/mozilla-inbound/rev/5313921e7dee
widget: Drop test check for OS X 10.6. r=RyanVM
Comment on attachment 9011314 [details]
Bug 1336712 - dom: Remove performance.now() test checks for Windows XP. r?bzbarsky

Boris Zbarsky [:bzbarsky, bz on IRC] has approved the revision.
Attachment #9011314 - Flags: review+
Attachment #9011318 - Attachment description: Bug 1336712 - toolkit: Drop test checks for OS X <= 10.8 and Windows XP. r?RyanVM → Bug 1336712 - toolkit: Drop test checks for OS X <= 10.8 and Windows XP. r?kmag

Comment 52

9 months ago
Pushed by cpeterson@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a902caa5f320
dom: Remove performance.now() test checks for Windows XP. r=bzbarsky

Comment 54

8 months ago
Pushed by cpeterson@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e006dd6e8fbd
toolkit: Drop test checks for OS X <= 10.8 and Windows XP. r=kmag
Status: ASSIGNED → RESOLVED
Closed: 8 months ago
Keywords: leave-open
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.