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

add acceptance tests #27

Merged
merged 10 commits into from
Aug 30, 2017
Merged

add acceptance tests #27

merged 10 commits into from
Aug 30, 2017

Conversation

tomwayson
Copy link
Member

@tomwayson tomwayson commented Aug 29, 2017

While ember t, ember t -s, and npm test all work with this there is a major limitation:

We can't run more than one acceptance test b/c the application route's renderTemplate() gets called for each, which causes esri-loader to throw an "already loaded" error, which causes ember-cli-qunit to fail the test, even if that error is handled in a catch() - are you f'ing kidding me?

I've tried the suggested work arounds in that thread. Using .then(() => {}, err => {}) instead of .catch() didn't seem to work (perhaps b/c the underlying error is thrown and passed via callback instead of a promise?). I was able to overwrite Ember.Test.adapter.exception, but it doesn't work for the first run of ember t -s, only subsequent runs. Also it's 🦇 💩 crazy to do that for every acceptance test.

Also, I suspect that we need to wrap this line in an Ember.run() as well, but that's just not exercised by this test.

All that said, I'm PRing this code as is, b/c tests actually pass (on my machine, at least). I'll open another [DNM] PR (#28) that better demonstrates the above issues, but also introduces others.

…mber.run()

also handles case where JSAPI is loaded before assertions are run
@tomwayson tomwayson changed the title add an acceptance test; get it to pass by wrapping load callback w/ E… add an acceptance test that passes by wrapping load callback w/ Ember.run() Aug 29, 2017
tests fail (sometimes) in odd ways:
- `ember t` always seems to pass
- `ember t -s` fails the first run with phantom showing:

Global error: TypeError: undefined is not an object (evaluating '_qunit['default'].config.current.assert') at http://localhost:7357/assets/test-support.js, line 18229

JSHint | unit/services/esri-loader-test.js: global failure
    ✘ TypeError: undefined is not an object (evaluating '_qunit['default'].config.current.assert')
        http://localhost:7357/assets/test-support.js:18229
TypeError: undefined is not an object (evaluating '_qunit['default'].config.current.assert') at http://localhost:7357/assets/test-support.js, line 18229

and chrome showing:

JSHint | unit/services/esri-loader-test.js: global failure (1, 0, 1)
Uncaught TypeError: Cannot read property 'assert' of undefined

- subsequent runs (pressing enter in console, saving a file, reloading chrome) fail consistently in phantom, but sometimes pass in chrome
also handles case where JSAPI is loaded before assertions are run
lot's of issues w/ this as it stands:
- can't run more than one acceptance test (see comments)
- phantom (only, not chrome) fails w/

Global error: Script error. at , line 0

JSHint | unit/services/esri-loader-test.js: global failure
    ✘ Script error.
        :0
Script error. at , line 0
phantom tests still blow up, but pass in Chrome (`ember t -s --launch=Chrome`)

also, still have to skip the index acceptance test
Copy link
Member

@mjuniper mjuniper left a comment

Choose a reason for hiding this comment

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

LGTM

… test

this code is _very_ ugly, but it works

will try to fix this upstream in esri-loader and remove the ugly hacks here
@tomwayson tomwayson changed the title add an acceptance test that passes by wrapping load callback w/ Ember.run() [DNM] add an acceptance test that passes by wrapping load callback w/ Ember.run() Aug 30, 2017
@tomwayson
Copy link
Member Author

tomwayson commented Aug 30, 2017

I've fixed the issue w/ running multiple tests in 0d9bd07 - #28 is starting to look like the PR to eventually merge

@tomwayson tomwayson changed the title [DNM] add an acceptance test that passes by wrapping load callback w/ Ember.run() add acceptance tests Aug 30, 2017
@tomwayson
Copy link
Member Author

Also resolves #9

Copy link
Member

@mjuniper mjuniper left a comment

Choose a reason for hiding this comment

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

LGTM

@mjuniper mjuniper merged commit d8d2402 into master Aug 30, 2017
@tomwayson tomwayson deleted the chore/acceptance-test branch August 30, 2017 21:05
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