Closed Bug 760735 Opened 12 years ago Closed 12 years ago

Write tests for Screen Orientation API

Categories

(Core :: DOM: Device Interfaces, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla16

People

(Reporter: gmealer, Assigned: gmealer)

References

()

Details

Attachments

(3 files, 1 obsolete file)

This is a work-in-progress. Posting to get some feedback re: functional programming style, since I'm relatively new to it. Some notes: The original code is in dom/base. I'm not at all sure where tests for that would properly go (the dom/base/test directory is pretty spare). I punted to dom/tests/marionette, but welcome correction. Right now, nothing in the test would fire a mozorientationchange event, so I'm keying order off either the emulator callback, or with an explicit setTimeout to pass control between functions. I'm unsure in cases where I've used setTimeout to chain test functions whether just doing a synchronous call would be legit too. I assume setTimeout keeps the stack from going too deep, but this is one of the areas where I'm a noob. I don't have tests yet for cycling the X and Y axes. My initial exploration showed some unexpected results doing that, and I want to loop in Mounir to find out if the current behavior is proper before writing tests to it. I have tests written for cycling through orientation locks, but they don't currently work, as you can only lock orientation as an app or a full-screen page. I need to find out how (if) I can get this test to run in a way that it can lock; then I can uncomment the rest of that function. I'll follow up presently with more about the unexpected results I got trying the other axis values, and post the testbed I used to get them.
Attachment #629441 - Flags: feedback?(philipp)
This is the testbed I used to see how mozOrientation behaved for different orientations of the emulator. You can see logs of the results at: http://people.mozilla.org/~gmealer/orientation/orientation.txt (full 360 degree sweep of all axes) http://people.mozilla.org/~gmealer/orientation/orientation-x.txt (360 degree sweep of just the x axis) http://people.mozilla.org/~gmealer/orientation/orientation-y.txt (360 degree sweep of just the y axis) http://people.mozilla.org/~gmealer/orientation/orientation-z.txt (360 degree sweep of just the z axis) I got some unusual behavior: Z axis didn't change orientation, which is expected (Z is towards center of the earth, i.e. up and down, so spinning around it doesn't change anything). X and Y axes only changed mozOrientation at the ~45 degree mark, which was unexpected. I would have expected one (whichever axis is flipping the device top-to-bottom on the axis parallel to the horizon) to change near the 90 degree mark to portrait-secondary, and again near 270 degree to portrait primary again. In other words, it'd change to upside down as it passed horizontal, and back as it passed again. I would have expected the other (whichever axis is rotating the device in the usual portrait->landscape way) to change to landscape-primary near 90, portrait-secondary near 180, landscape-secondary near 270, and back to portrait-primary near 360. Also, the values seem to be "sticky," as you can see by comparing the separate axis runs to the combined run. The portrait-secondary from X "stuck" at the beginning of the Y run, such that 0:0:0 was primary sometimes and secondary other times. I would have expected fairly deterministic values such that any given Z:X:Y is always a specific orientation. One possibility is that the emulator and the API don't play nicely somehow. One other possibility is I'm incorrect about the emulator values being degrees. I tried radians, though, and there was no change from 0-2pi in .5pi increments on any axis. Plus, the change at 45 suggests degrees, switching from mostly-vertical to mostly-horizontal. I'm not sure yet this is behavior is improper, so want some feedback from Mounir prior to moving forward. How should I be seeing this behave?
Attachment #629443 - Flags: feedback?(mounir)
Blocks: 673922
Comment on attachment 629443 [details] Testbed to display mozOrientation at different emulator orientations I think there are two things to test here: 1. the DOM API simply by using .mozLockOrientation/.mozOrientation/etc. This can be done with a mochitest (for Android and B2G only). Basically, you would check that calling lock sent the event and change the orientation value and in some situations you can't call lock, etc. 2. the backends (B2G only, using marionette): checking that for some sensor orientation values, the screen orientation is Foo or Bar. I can help you, give feedback and even revew (1) but you will have to find someone else for (2).
Attachment #629443 - Flags: feedback?(mounir)
Mounir, Yeah, I agree re: the breakdown. I'll fill in the rest of the lock tests (1) after I figure out how to make the test run as full screen or an app, so lock works. The problem is that the sensor orientation tests (2) are giving me unexpected results. I'm virtually spinning the phone in a complete circle on X, Y, and Z, and it's not traversing all four cardinal orientations on any axis. At least one axis should. I understand you can't give feedback on how the emulator is being used, but assuming I'm using it correctly can you clarify what the behavior of the Foo and Bar in question should be? Failing that, who can? I traced your code up to the point where you're getting the true orientation from the aConfiguration structure, but I'm unclear how/where that's getting filled in.
(In reply to Geo Mealer [:geo] from comment #4) > Mounir, > > Yeah, I agree re: the breakdown. I'll fill in the rest of the lock tests (1) > after I figure out how to make the test run as full screen or an app, so > lock works. You should use requestFullScreen() (see develeoper.mozilla.org). > The problem is that the sensor orientation tests (2) are giving me > unexpected results. > > I'm virtually spinning the phone in a complete circle on X, Y, and Z, and > it's not traversing all four cardinal orientations on any axis. At least one > axis should. > > I understand you can't give feedback on how the emulator is being used, but > assuming I'm using it correctly can you clarify what the behavior of the Foo > and Bar in question should be? > > Failing that, who can? I traced your code up to the point where you're > getting the true orientation from the aConfiguration structure, but I'm > unclear how/where that's getting filled in. As I said, I really have no idea how X,Y,Z should be related to orientation. This is fully up to the backend. On Android, we should heavily test that. Maybe one very simple position for each orientation. On B2G, you should ask the assignee of bug 743638. I think you should open another bug to test the B2G backend.
OK, so it sounds like I've conflated the back end and your API. If I understand the responsibilities correctly, then: 1) The back end is responsible for translating an X, Y, Z into a simplified orientation that you consume. 2) Your API is responsible for sending out the mozorientationchange event when the simplified orientation changes. 3) Your API is also responsible for locking/unlocking. Is that correct?
The DOM level is responsible of: - the rules regarding when the lock method can be called; - the rules regarding which orientation names can be used; - asking the backend to lock/unlock and current state of the screen orientation; - dispatching the orientation change event to the content (from the backend's input).
So the good news is that testing this on-device shows that orientation translation and events work correctly in the final stack. Testbed for that is here: http://people.mozilla.org/~gmealer/exercises/orientation . So, there's something wonky in how I'm attempting to use the emulator, or in how it behaves. I'm not sure yet if this means we can't test orientation this way or not. I'll do some tests that try a very full range of values in the emulator to see if I can gain understanding of how it's using the {*:*:*} values you use to set the orientation sensors. In the meantime, I'll separate out the tests that don't need emulator (sanity check and negatives) into mochitests and get a patch up to get into CI. I expect that to be done on Monday. I have not yet been able to test locking; in emulator, it's blocked by Bug 761856, and on-device the browser doesn't seem to allow full-screen content at all. I may have to put my code into a standalone app to test it.
(In reply to Geo Mealer [:geo] from comment #8) > I have not yet been able to test locking; in emulator, it's blocked by Bug > 761856, and on-device the browser doesn't seem to allow full-screen content > at all. I may have to put my code into a standalone app to test it. The Firefox on Android should allow locking and fullscreen.
Comment on attachment 629441 [details] [diff] [review] (WIP) First pass at Marionette tests for Screen Orientation API Review of attachment 629441 [details] [diff] [review]: ----------------------------------------------------------------- Looks good superficially. I don't know this API well enough to go beyond that.
Attachment #629441 - Flags: feedback?(philipp) → feedback+
(In reply to Philipp von Weitershausen [:philikon] from comment #10) > Comment on attachment 629441 [details] [diff] [review] > (WIP) First pass at Marionette tests for Screen Orientation API > > Review of attachment 629441 [details] [diff] [review]: > ----------------------------------------------------------------- > > Looks good superficially. I don't know this API well enough to go beyond > that. No worries, I didn't expect domain expertise. Was more concerned about my use of asynchronous functions, since my background is mostly structured/OOP, not functional. Thanks!
As mentioned before, I've moved basic negative and sanity testing into mochitest so it can run widely. I haven't put positive tests in here. I have no idea if it's legit for me to attempt to fullscreen during a mochitest run, as locking requires. I'd also need to put in platform selectors as desktop in particular doesn't support locking. If it looks feasible, I'll add them later.
Attachment #633754 - Flags: review?(mounir)
Comment on attachment 633754 [details] [diff] [review] Mochitest for negative/sanity tests of Screen Orientation API Review of attachment 633754 [details] [diff] [review]: ----------------------------------------------------------------- r=me with the change I suggested below and also, move the test here: dom/base/test/test_screen_orientation.html ::: dom/tests/mochitest/orientation/test_bug760735.html @@ +32,5 @@ > +try { > + var initialOrientation = window.screen.mozOrientation; > + > + // Sanity tests > + ok(initialOrientation !== undefined, "mozOrientation is defined (value=" + initialOrientation + ")"); You could check that the orientation is one of those: ["portrait-primary", "portrait-secondary", "landscape-primary", "landscape-secondary"] You can do that with: var allowedOrientations = ["portrait-primary", "portrait-secondary", "landscape-primary", "landscape-secondary"]; isnot(allowedOrientations.indexOf(initialOrientation), -1, "...");
Attachment #633754 - Flags: review?(mounir) → review+
And thank you for writing this test :)
Has the r=me changes requested. Resubmitting for review because I don't have level 1 commit, so can't land the patch myself per r=me. Also, no problem. It's great to be helping out directly!
Attachment #633754 - Attachment is obsolete: true
Attachment #635493 - Flags: review?(mounir)
Comment on attachment 635493 [details] [diff] [review] Revised mochitests with mounir's requested changes Review of attachment 635493 [details] [diff] [review]: ----------------------------------------------------------------- r=me You said you do not have level 1 commit access so you can't push but you actually need level 3 commit access to push to m-i or m-c. Do you really don't have level 1 commit access or level 3 commit access ? I can vouch you for the level 1 commit access. Just open a bug to request it and CC me.
Attachment #635493 - Flags: review?(mounir) → review+
Mounir: sorry, I misspoke. I have level 2 access from my other automation efforts. I did mean mozilla-central/level 3. For some reason when I wrote that I was thinking 1=higher, not 3=higher. Thanks for the voucher offer!
Pushed to mozilla-inbound.
Status: NEW → ASSIGNED
Flags: in-testsuite+
Target Milestone: --- → mozilla16
Please post a changeset link when pushing to inbound... https://hg.mozilla.org/integration/mozilla-inbound/rev/d592966ede4f
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: