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

Update ember-testing tests to new syntax #16022

Merged
merged 3 commits into from
Dec 30, 2017

Conversation

cibernox
Copy link
Contributor

Part of #15988

['@test asyncStart calls stop'](assert) {
var originalStop = QUnit.stop;
try {
QUnit.stop = function() {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not very sure what to do about QUnit.stop and QUnit.start

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The places that are explicitly testing asyncStart and asyncStop should continue to use QUnit.stop / QUnit.start for now, but anywhere else it is being done should be migrated to assert.async().

} finally {
QUnit.start = originalStart;
['@test asyncEnd calls start'](assert) {
var originalStart = QUnit.start;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again

var error = { err: 'hai' };
var originalOk = window.ok;
try {
window.ok = function(val, msg) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is also unclear.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have some tests for this in ember-qunit, basically you should do:

    let originalPushResult = assert.pushResult;
    assert.pushResult = function(resultInfo) {
      // Inverts the result so we can test failing assertions
      resultInfo.result = !resultInfo.result;
      resultInfo.message = `Failed: ${resultInfo.message}`;
      originalPushResult(resultInfo);
    };

return new RSVP.Promise(function(resolve) {
QUnit.stop(); // raw async, we must inform the test framework manually
setTimeout(function() {
QUnit.start(); // raw async, we must inform the test framework manually
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

again

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should use assert.async(); here

@rwjblue
Copy link
Member

rwjblue commented Dec 30, 2017

@cibernox - Had any time to work on this?

@cibernox
Copy link
Contributor Author

I do now. TBH, i just forgot about this PR 😅

@cibernox cibernox force-pushed the upgrade-syntax-ember-testing-tests branch from d7be5d5 to b9a40bb Compare December 30, 2017 17:22
@@ -18,6 +18,6 @@ export default Adapter.extend({
QUnit.start();
},
exception(error) {
ok(false, inspect(error));
ok(false, inspect(error)); // How do we access assert.ok here?
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rwjblue Should I use QUnit.assert.ok here instead?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QUnit.config.current.assert.ok

@cibernox cibernox force-pushed the upgrade-syntax-ember-testing-tests branch from bc34812 to ca004d3 Compare December 30, 2017 17:51
@cibernox
Copy link
Contributor Author

@rwjblue ready

@rwjblue rwjblue merged commit 85e1084 into emberjs:master Dec 30, 2017
@cibernox cibernox deleted the upgrade-syntax-ember-testing-tests branch December 31, 2017 10:17
@thoov thoov mentioned this pull request Jan 5, 2018
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants