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

Jump to bottom

Remove exception method from QUnit test adapter. #307

Merged
merged 1 commit into from Jan 17, 2018
Merged

Remove exception method from QUnit test adapter. #307

merged 1 commit into from Jan 17, 2018

Conversation

rwjblue
Copy link
Member

@rwjblue rwjblue commented Dec 18, 2017

As of Ember 2.17 the Ember.Test.adapter is not required to have an exception method, and the method essentially has no purpose. The default implementation (in the Ember.Test.QUnitAdapter) is to re-throw, but that is exactly what happens by default.

This removes "yet another magical testing thing", and relies on both Ember and QUnit's default behaviors to ensure that tests properly fail when unhandled RSVP rejections occur.

Cross linking (again) emberjs/ember.js#15871.

Should probably wait for the tests added around unhandled rejections in #306.

@rwjblue
Remove exception method from QUnit test adapter.
d651517 
As of Ember 2.17 the `Ember.Test.adapter` is not required to have an
`exception` method, and the method essentially has no purpose. The
default implementation (in the `Ember.Test.QUnitAdapter`) is to
re-throw, but that is _exactly_ what happens by default.

This removes "yet another magical testing thing", and relies on both
Ember and QUnit's default behaviors to ensure that tests properly fail
when unhandled RSVP rejections occur.
Turbo87
Turbo87 reviewed Dec 18, 2017
View reviewed changes
addon-test-support/ember-qunit/adapter.js
@@ -1,5 +1,6 @@
import Ember from 'ember';
import QUnit from 'qunit';
import hasEmberVersion from '@ember/test-helpers/has-ember-version';
Copy link
Member

Choose a reason for hiding this comment

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

is that helper public API?

Copy link
Member Author

Choose a reason for hiding this comment

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

Its effectively public, its basically in the same state as wait was before. Lots of folks use it, but we haven't officially said its "public".

I'll open a PR to export it from the entry point...

addon-test-support/ember-qunit/adapter.js
// method When `exception` is not present, the unhandled rejection is
// automatically re-thrown and will therefore hit QUnit's own global error
// handler (therefore appropriately causing test failure)
if (!hasEmberVersion(2, 17)) {
Copy link
Member

Choose a reason for hiding this comment

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

this condition sounds like it would only work for 2.17, but I'm assuming the same is true for 2.18?

Copy link
Member Author

Choose a reason for hiding this comment

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

hasEmberVersion is basically like semver.gte for the provided major + minor.

@rwjblue rwjblue merged commit 5382730 into master Jan 17, 2018
@rwjblue rwjblue deleted the remove-adapter-exception branch January 17, 2018 15:52
rwjblue added a commit to ember-cli/ember-cli-qunit that referenced this pull request Jan 17, 2018
@rwjblue
Update minimum version of ember-qunit to 3.3.0.
371b9fa 
Main changes:

* [#307](emberjs/ember-qunit#307) Remove `exception` method from QUnit test adapter when using Ember 2.17+.  ([@rwjblue](https://github.com/rwjblue))
* [#306](emberjs/ember-qunit#306) Update to QUnit 2.5.0. ([@rwjblue](https://github.com/rwjblue))
  * Adds `assert.rejects`.
  * Reduces the delays around `assert.async` and returning promises from hook methods.
  * Adds a native promise unhandled rejection handler.
  * Fixes an issue with `notrycatch` mode being ignored for promises returned (e.g. it would previously always `.catch` every returned promise).
* [#304](emberjs/ember-qunit#304) Add validation for `Ember.onerror` in testing. ([@rwjblue](https://github.com/rwjblue))
Turbo87 pushed a commit to ember-cli/ember-cli-eslint that referenced this pull request Oct 17, 2018
@dependabot @Turbo87
Bump ember-cli-qunit from 4.0.1 to 4.4.0 (#280)
031c57b 
Bumps [ember-cli-qunit](https://github.com/ember-cli/ember-cli-qunit) from 4.0.1 to 4.4.0.
<details>
<summary>Changelog</summary>

*Sourced from [ember-cli-qunit's changelog](https://github.com/ember-cli/ember-cli-qunit/blob/master/CHANGELOG.md).*

> # Change Log
> 
> ## v4.3.2 (2018-02-16)
> 
> #### 🐛 Bug Fix
> * Update ember-qunit to 3.3.2 (fixing an issue with `waitUntil`)
> 
> #### Committers: 1
> - Robert Jackson ([rwjblue](https://github.com/rwjblue))
> 
> ## v4.3.1 (2018-02-11)
> 
> #### 🚀 Enhancement
> * [#204](https://github-redirect.dependabot.com/ember-cli/ember-cli-qunit/pull/204) Update dependencies to latest versions. ([[**rwjblue**](https://github.com/rwjblue)](https://github.com/rwjblue))
> 
> #### 🏠 Internal
> * [#200](https://github-redirect.dependabot.com/ember-cli/ember-cli-qunit/pull/200) Update RELEASE.md for lerna-changelog & travis deploy. ([[**kategengler**](https://github.com/kategengler)](https://github.com/kategengler))
> 
> #### Committers: 2
> - Katie Gengler ([kategengler](https://github.com/kategengler))
> - Robert Jackson ([rwjblue](https://github.com/rwjblue))
> 
> ## v4.3.0 (2018-01-17)
> 
> #### 🐛 Bug Fix
> * Update minimum version of ember-qunit to 3.3.0.
>   * [emberjs/ember-qunit#307](https://github-redirect.dependabot.com/emberjs/ember-qunit/pull/307) Remove `exception` method from QUnit test adapter when using Ember 2.17+.  ([[**rwjblue**](https://github.com/rwjblue)](https://github.com/rwjblue))
>   * [emberjs/ember-qunit#306](https://github-redirect.dependabot.com/emberjs/ember-qunit/pull/306) Update to QUnit 2.5.0. ([[**rwjblue**](https://github.com/rwjblue)](https://github.com/rwjblue))
>     * Adds `assert.rejects`.
>     * Reduces the delays around `assert.async` and returning promises from hook methods.
>     * Adds a native promise unhandled rejection handler.
>     * Fixes an issue with `notrycatch` mode being ignored for promises returned (e.g. it would previously always `.catch` every returned promise).
>   * [emberjs/ember-qunit#304](https://github-redirect.dependabot.com/emberjs/ember-qunit/pull/304) [BREAKING BUGFIX] Add validation for `Ember.onerror` in testing. ([[**rwjblue**](https://github.com/rwjblue)](https://github.com/rwjblue))
> 
> ## v4.2.1 (2017-12-18)
> 
> #### 🐛 Bug Fix
> * [#198](https://github-redirect.dependabot.com/ember-cli/ember-cli-qunit/pull/198) Update minimum version of ember-qunit. ([[**rwjblue**](https://github.com/rwjblue)](https://github.com/rwjblue))
>   * Ensures re-exports from ember-test-helpers are still present (with a deprecation).
>   * Ensures that `Ember.testing` is set properly from "normal" non `moduleFor*` and/or `setup*Test` style tests.
> 
> #### 🏠 Internal
> * [#197](https://github-redirect.dependabot.com/ember-cli/ember-cli-qunit/pull/197) Update ember-cli blueprint to 2.17.1.. ([[**rwjblue**](https://github.com/rwjblue)](https://github.com/rwjblue))
> 
> #### Committers: 1
> - Robert Jackson ([rwjblue](https://github.com/rwjblue))
> 
> ## v4.2.0 (2017-12-17)
> 
> #### 🚀 Enhancement
></table> ... (truncated)
</details>
<details>
<summary>Commits</summary>

- [`4ad1af2`](ember-cli/ember-cli-qunit@4ad1af2) release v4.4.0 🎉
- [`f4bd36b`](ember-cli/ember-cli-qunit@f4bd36b) Merge pull request [#212](https://github-redirect.dependabot.com/ember-cli/ember-cli-qunit/issues/212) from ember-cli/upgrade
- [`f32d88a`](ember-cli/ember-cli-qunit@f32d88a) bump ember-qunit
- [`63ac2f2`](ember-cli/ember-cli-qunit@63ac2f2) Merge pull request [#210](https://github-redirect.dependabot.com/ember-cli/ember-cli-qunit/issues/210) from samselikoff/upgrade-ember-qunit
- [`300b36c`](ember-cli/ember-cli-qunit@300b36c) Upgrade ember-qunit
- [`d3dc0f4`](ember-cli/ember-cli-qunit@d3dc0f4) v4.3.2
- [`f4897da`](ember-cli/ember-cli-qunit@f4897da) Add v4.3.2 to CHANGELOG.md.
- [`c3a2396`](ember-cli/ember-cli-qunit@c3a2396) Update ember-qunit to v3.3.2.
- [`ac4ea28`](ember-cli/ember-cli-qunit@ac4ea28) v4.3.1
- [`57b348a`](ember-cli/ember-cli-qunit@57b348a) Add v4.3.1 to CHANGELOG.md.
- Additional commits viewable in [compare view](ember-cli/ember-cli-qunit@v4.0.1...v4.4.0)
</details>
<br />

[![Dependabot compatibility score](https://api.dependabot.com/badges/compatibility_score?dependency-name=ember-cli-qunit&package-manager=npm_and_yarn&previous-version=4.0.1&new-version=4.4.0)](https://dependabot.com/compatibility-score.html?dependency-name=ember-cli-qunit&package-manager=npm_and_yarn&previous-version=4.0.1&new-version=4.4.0)

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`.

Dependabot will **not** automatically merge this PR because it includes a minor update to a development dependency.

---

**Note:** This repo was added to Dependabot recently, so you'll receive a maximum of 5 PRs for your first few update runs. Once an update run creates fewer than 5 PRs we'll remove that limit.

You can always request more updates by clicking `Bump now` in your [Dependabot dashboard](https://app.dependabot.com).

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot ignore this [patch|minor|major] version` will close this PR and stop Dependabot creating any more for this minor/major version (unless you reopen the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
- `@dependabot use these labels` will set the current labels as the default for future PRs for this repo and language
- `@dependabot use these reviewers` will set the current reviewers as the default for future PRs for this repo and language
- `@dependabot use these assignees` will set the current assignees as the default for future PRs for this repo and language
- `@dependabot use this milestone` will set the current milestone as the default for future PRs for this repo and language
- `@dependabot badge me` will comment on this PR with code to add a "Dependabot enabled" badge to your readme

Additionally, you can set the following in your Dependabot [dashboard](https://app.dependabot.com):
- Update frequency (including time of day and day of week)
- Automerge options (never/patch/minor, and dev/runtime dependencies)
- Pull request limits (per update run and/or open at any time)
- Out-of-range updates (receive only lockfile updates, if desired)
- Security updates (receive only security updates, if desired)

Finally, you can contact us by mentioning @dependabot.

</details>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Turbo87 Turbo87 Turbo87 left review comments

Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@rwjblue @Turbo87