Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Import test cases from DVCS for Device Orientation Event #350

Merged

Conversation

@zqzhang
Copy link
Contributor

@zqzhang zqzhang commented Sep 24, 2013

@hoppipolla-critic-bot
Copy link

@hoppipolla-critic-bot hoppipolla-critic-bot commented Sep 24, 2013

Critic review: https://critic.hoppipolla.co.uk/r/330

This is an external review system which you may optionally use for the code review of your pull request.

x = gravity.x;
// Y : angle par rapport a y
y = gravity.y;
// Z : acceleromètre (de base : -9.81)

This comment has been minimized.

@tobie

tobie Sep 24, 2013
Contributor

In French, really?

function init(){
if (window.DeviceMotionEvent) {
window.addEventListener('devicemotion', DeviceMotionHandler, false);
alert("Launch Test");

This comment has been minimized.

@tobie

tobie Sep 24, 2013
Contributor

This instruction is unclear.

@tobie
Copy link
Contributor

@tobie tobie commented Sep 24, 2013

It seems most of these tests are manual and should be marked as such (-manual file suffix.)

<script src="http://w3c-test.org/resources/testharness.js"></script>
<script src="http://w3c-test.org/resources/testharnessreport.js"></script>
<script src="http://w3c-test.org/resources/WebIDLParser.js"></script>
<script src="http://w3c-test.org/resources/idlharness.js"></script>

This comment has been minimized.

@tobie

tobie Sep 24, 2013
Contributor

Don't think idlharness or WebIDLParser are necessary here.

ewin = null;
window.addEventListener('devicemotion', function(e) {etype = e.type; ewin = this;}, false);

setTimeout("testing()", 200);

This comment has been minimized.

@tobie

tobie Sep 24, 2013
Contributor

This is terribly ugly (relies on eval + globals for state).

The timeout is also very brittle.

We should seek a better option here, maybe:

var t1 = async_test("The corresponding event must be of type DeviceMotionEvent");
var t2 = async_test("The corresponding event must fire on the window object");
var run = false;
window.addEventListener('devicemotion', function(e) {
    if (!run) {
        run = true;
        var _this = this;
        t1.step(function() { assert_equals(e.type, "devicemotion"); });
        t1.done();
        t2.step(function() { assert_equals(_this, window); });
        t2.done();
    }
}, false);
}, false);
alert("Put your device on a horizontal surface with the screen upmost.");

tx = async_test("X angle must be set to zero");

This comment has been minimized.

@tobie

tobie Sep 24, 2013
Contributor

globals :(

</div>

<script>
setup({timeout:1500});

This comment has been minimized.

@tobie

tobie Sep 24, 2013
Contributor

No need to indicate short timeouts like this.

@@ -0,0 +1,167 @@
<!DOCTYPE html>

This comment has been minimized.

@tobie

tobie Sep 24, 2013
Contributor

You might want to turn parts of this into a README.

@zqzhang
Copy link
Contributor Author

@zqzhang zqzhang commented Sep 25, 2013

@tobie, I just copied test files here and will review and update them soon.

@tobie
Copy link
Contributor

@tobie tobie commented Sep 25, 2013

Oh, OK. In the future, please only send pull requests when the work is
ready to be reviewed. Thanks!

Zhiqiang Zhang added 2 commits Oct 14, 2013
- Turn device_orientation_api.html and manifest.txt files into mainifest.txt and TODO.txt
- Remove t026.html as it has same checkpoints as screen-upmost.html, and merge values-null.html into screen-upmost.html
- Remove t030.html, t031.html, t032.html as they are incorrect in null attributes checking
- Rewrite other test cases
@darobin
Copy link
Contributor

@darobin darobin commented Jan 23, 2014

May you please:

  • Rename the manual tests as -manual (most are)
  • Drop the manifest.txt
  • Remove all usage of user-scalable=no

Thanks!

- Rename the manual tests as -manual
- Drop the manifest.txt
- Remove all usage of user-scalable=no
@zqzhang
Copy link
Contributor Author

@zqzhang zqzhang commented Jan 24, 2014

Hi @tobie and @darobin, these tests look good to me now after my review and update. Could you please review them? Many thanks.

@sideshowbarker
Copy link
Member

@sideshowbarker sideshowbarker commented May 28, 2014

This is waiting on review that @zqzhang asked for back in January. @tobie @darobin anybody?

cvrebert added a commit to whatwg/platform.html5.org that referenced this pull request Sep 15, 2015
</head>
<body>
<p>Free fall the device to run the test, with the screen horizontal and upmost.</p>
<div id="log"></div>

This comment has been minimized.

@Honry

Honry May 23, 2016
Contributor

Indent not consistent.

This comment has been minimized.

@Honry

Honry May 23, 2016
Contributor

Please fix all indent issue in other files.

}
}, false);

alert("Put the device on a horizontal surface.");

This comment has been minimized.

@Honry

Honry May 23, 2016
Contributor

Better to convert this hint to a text tag.

run = true;
gamma = e.gamma; // Gamma : angle par rapport a x
beta = e.beta; // Beta : angle par rapport a y
alpha = e.alpha; // Alpha : orientation (N-S-E-O)

This comment has been minimized.

@Honry

Honry May 23, 2016
Contributor

alpha in this test is unused. Need an assertion for it.

- Indent consistently using spaces
- Put user interaction instructions into a paragraph
- Remove unreferenced variable
@zqzhang
Copy link
Contributor Author

@zqzhang zqzhang commented Jun 13, 2016

@Honry thanks for the review. Your comments shall be addressed. Please take another look, thanks.

@Honry
Copy link
Contributor

@Honry Honry commented Jun 13, 2016

LGTM

@zqzhang zqzhang merged commit 7b96cc0 into web-platform-tests:master Jun 13, 2016
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@zqzhang zqzhang deleted the zqzhang:submission/zqzhang/orientation-event branch Jun 13, 2016
arronei added a commit to arronei/web-platform-tests that referenced this pull request Jun 14, 2016
…m-tests#350)

* Import test cases from DVCS for Device Orientation Event

https://dvcs.w3.org/hg/geo/rev/5ee2a7297634

* Update test cases for DeviceOrientation Event

- Turn device_orientation_api.html and manifest.txt files into mainifest.txt and TODO.txt
- Remove t026.html as it has same checkpoints as screen-upmost.html, and merge values-null.html into screen-upmost.html
- Remove t030.html, t031.html, t032.html as they are incorrect in null attributes checking
- Rewrite other test cases

* Remove http://w3c-test.org from test files.

* Address @darobin comments:

- Rename the manual tests as -manual
- Drop the manifest.txt
- Remove all usage of user-scalable=no

* Use WebIDLParser.js

* Address @Honry comments

- Indent consistently using spaces
- Put user interaction instructions into a paragraph
- Remove unreferenced variable

* Remove trailing whitespace

* Remove TODO as tests shall follow the latest spec at

https://w3c.github.io/deviceorientation/spec-source-orientation.html
ivanzr added a commit to ivanzr/web-platform-tests that referenced this pull request Jun 29, 2016
…m-tests#350)

* Import test cases from DVCS for Device Orientation Event

https://dvcs.w3.org/hg/geo/rev/5ee2a7297634

* Update test cases for DeviceOrientation Event

- Turn device_orientation_api.html and manifest.txt files into mainifest.txt and TODO.txt
- Remove t026.html as it has same checkpoints as screen-upmost.html, and merge values-null.html into screen-upmost.html
- Remove t030.html, t031.html, t032.html as they are incorrect in null attributes checking
- Rewrite other test cases

* Remove http://w3c-test.org from test files.

* Address @darobin comments:

- Rename the manual tests as -manual
- Drop the manifest.txt
- Remove all usage of user-scalable=no

* Use WebIDLParser.js

* Address @Honry comments

- Indent consistently using spaces
- Put user interaction instructions into a paragraph
- Remove unreferenced variable

* Remove trailing whitespace

* Remove TODO as tests shall follow the latest spec at

https://w3c.github.io/deviceorientation/spec-source-orientation.html
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants