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

[DNM] Chore/map acceptance test #28

Merged
merged 8 commits into from
Aug 30, 2017

Conversation

tomwayson
Copy link
Member

@tomwayson tomwayson commented Aug 29, 2017

This PR suffers from the same problems in #27 - we still can't run more than a single acceptance test.

In addition, by trying to actually test the map, we now get the following error every time in PhantomJS (but not in Chrome):

Global error: Script error. at , line 0

JSHint | unit/services/esri-loader-test.js: global failure
    ✘ Script error.
        :0
Script error. at , line 0

NOTE: that error was happening in ad5c133 - when the acceptance test didn't even try to wait for and inspect the map. In other words, the error occurs simply by loading the JSAPI modules, not even using them. Which sucks, b/c it was such a royal PIA to write a test helper to wait for the map to load.

I'm so f'ing over Ember acceptance tests and the JSAPI.

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
… 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
Copy link
Member Author

As of 0d9bd07 we can run more than one acceptance test.

I tried DRYing up the error handling code but was having problems. Besides, I ultimately want to move that code upstream into esri-loader in Esri/esri-loader#28

I'm still seeing the global script error in phantom. I'm going to try moving the loadModules code into the route model() hooks in hopes that that fixes it. If that doesn't I'll configure the tests to only run in Chrome, maybe FF or headless Chrome in CI and we can merge this.

@tomwayson
Copy link
Member Author

tomwayson commented Aug 30, 2017

I would have had to re-arrange most of the dummy app to run loadModules() in the route hook, so I abandoned that, and any hope of running tests in PhantomJS. Phantom's so 2016 anyway.

I configured the tests to run in FF in CI now. Both ember t and npm test pass locally in FF and they've passed in Travis, so I'm going to merge this PR and and assume that will update #27 and we can merge to master from there..

@tomwayson tomwayson merged commit 56ff2cf into chore/acceptance-test Aug 30, 2017
@tomwayson tomwayson deleted the chore/map-acceptance-test branch August 30, 2017 20:19
@dmfenton
Copy link

@tomwayson I wouldn't worry about phantom. It's a deprecated library and I'd expect most folks to be moving towards headless chrome at this point.

@tomwayson
Copy link
Member Author

@dmfenton speaking of... #30 has DMF written all over it. 😉

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