Closed Bug 1121327 Opened 5 years ago Closed 5 years ago

OSX variable in reftest condition sandbox needs to handle 10.10 (Yosemite) better

Categories

(Testing :: Reftest, defect)

x86_64
macOS
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla38

People

(Reporter: dbaron, Assigned: dbaron)

References

Details

Attachments

(2 files)

Bug 789771 added an OSX refvaiable to the reftest condition sandbox, whose value is a float with the OS X version.

This can be used in conditions like fails-if(OSX<10.8).

On 10.10 (Yosemite), however, it has the value 10.1.  We should probably change the variable to OSX10 and the value to the bit after the float, as an integer.
Alternately, perhaps strip out the dot and just make it an integer (with a leading zero for the minor version) like 10.8 -> 1008, 10.10 -> 1010.
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #2)
> Alternately, perhaps strip out the dot and just make it an integer (with a
> leading zero for the minor version) like 10.8 -> 1008, 10.10 -> 1010.

Yeah, that's probably better.
Assignee: nobody → dbaron
Status: NEW → ASSIGNED
Using undefined has the advantage that we can use < and > tests with the
OSX variable.  (We currently have no such tests in the tree, perhaps
partly because they didn't work with non-OSX being 0.)
Attachment #8549387 - Flags: review?(ted)
Comment on attachment 8549387 [details] [diff] [review]
patch 1 - Make the OSX variable in the reftest condition sandbox be an integer (1006, 1010) so that it scales for 10.10, and undefined for non-Mac rather than 0

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

::: layout/tools/reftest/reftest.js
@@ +669,5 @@
>      httpProps.forEach((x) => sandbox.http[x] = hh[x]);
>  
> +    // Set OSX to be the Mac OS X version, as an integer, or undefined
> +    // for other platforms.  The integer is formed by 100 times the
> +    // major verion plus the minor version, so 1006 for 10.6, 1010 for

typo: version
Attachment #8549387 - Flags: review?(ted) → review+
Comment on attachment 8549388 [details] [diff] [review]
patch 2 - Update reftest and crashtest manifests for new OSX variable in condition sandbox

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

Given that we don't have tests running on anything >10.8 in automation (10.10 was just stood up on Cedar but is not green yet) I suspect a lot of these "OSX==1008" actually want to be "OSX>1008" and likewise "OSX==1007||OSX==1008" might really want to be "OSX>1007".
Attachment #8549388 - Flags: review?(ted) → review+
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #8)
> Given that we don't have tests running on anything >10.8 in automation
> (10.10 was just stood up on Cedar but is not green yet) I suspect a lot of
> these "OSX==1008" actually want to be "OSX>1008" and likewise
> "OSX==1007||OSX==1008" might really want to be "OSX>1007".

Agreed; that's the next step.
https://hg.mozilla.org/mozilla-central/rev/6084da03dcd8
https://hg.mozilla.org/mozilla-central/rev/4323060230fb
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
You need to log in before you can comment on or make changes to this bug.