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

Test ES6 code in PhantomJS #3003

Closed
bajtos opened this issue Dec 6, 2016 · 7 comments
Closed

Test ES6 code in PhantomJS #3003

bajtos opened this issue Dec 6, 2016 · 7 comments
Assignees

Comments

@bajtos
Copy link
Member

bajtos commented Dec 6, 2016

Right now, we use PhantomJS headless browser to run (some of) loopback tests in a browser environment. Unfortunately, PhantomJS does not support ES6 yet - see ariya/phantomjs#14506

To allow us to write ES6 code, we need to add some sort of ES6-to-ES5 transpiler to our Grunt pipeline.

@ritch
Copy link
Member

ritch commented Dec 6, 2016

@bajtos anyone else able to take this on?

@kjdelisle @pthieu ??

@bajtos
Copy link
Member Author

bajtos commented Dec 7, 2016

anyone else able to take this on?

Ah, sure, of course.

So far, we are using browserify to build the browser bundle (see test/karma.conf.js#L19) and adding es5-shim to add missing ES5 APIs (see test/karma.conf.js#L23).

Unfortunately, ES6 is not only about extra APIs, we need to add ES6-to-ES5 transpiler to the mix. I think it should be enough to configure karma-browserify to transform the sources (transpile, perhaps using Babel?), the plugin already supports custom transformations - see https://github.com/nikku/karma-browserify. We may need to add karma-es6-shim to the mix too.

That's a quick intro. I didn't have time to look into this issue, so take my comment with a grain of salt.

@bajtos bajtos changed the title Testing ES6 code in PhantomJS Test ES6 code in PhantomJS Dec 8, 2016
@pthieu
Copy link

pthieu commented Dec 8, 2016

I may be able to take this on. Have transpiled and used grunt in production; but not together with tests. Might need some hand-holding. What's the priority on this?
@ritch @bajtos ^

@superkhau
Copy link
Contributor

IMO, this is very important to having the loopback repo pass (I think there are windows issues related to phantom too).

@bajtos
Copy link
Member Author

bajtos commented Dec 9, 2016

What's the priority on this?

I would prefer to get this done ASAP. We have already spent a lot of time and effort to allow us to use ES6 in our code, I think this is the last missing piece.

On the other hand, this is not related to PhantomJS passing/failing in the current code, therefore it's not a critical problem.

I have explicitly disabled ES6 syntax in eslint config of loopback. For upstream dependencies like strong-remoting and loopback-datasource-juggler, we are relying on downstream CI builds to detect the situation where an ES6 code would break PhantomJS tests.

@pthieu pthieu assigned pthieu and unassigned pthieu Dec 13, 2016
@bajtos
Copy link
Member Author

bajtos commented Dec 15, 2016

@pthieu

cloned loopback, npm install && npm test

  757 passing (22s)
  13 pending

not entirely sure how to verify what should be failing and should then be passing after transpile.

The tests are passing because we are not using any ES6 yet. Here are the instructions how to reproduce the problem we need to fix:

  1. Enable ES6 in eslint config - remove these three lines. eslint-config-loopback allows ES6 by default, these three lines were overriding that setting to allow ES5 only.
  2. Add some ES6 construct to the code, for example use const instead of var in index.js
  3. npm test (or grunt karma:unit-once to get results faster) - you should see a failure now.
[...]
15 12 2016 12:25:41.165:INFO [launcher]: Starting browser PhantomJS
15 12 2016 12:25:41.974:INFO [PhantomJS 2.1.1 (Mac OS X 0.0.0)]: Connected on socket /#pDz5xw48U7Ye01XEAAAA with id 65726246
PhantomJS 2.1.1 (Mac OS X 0.0.0) ERROR
  SyntaxError: Unexpected token 'const'
  at /var/folders/32/1zm3qsc94pzddjq0d75pwvzr0000gn/T/889fd1dd6c1ce0c8eaf70f3324e84272.browserify:4172
PhantomJS 2.1.1 (Mac OS X 0.0.0): Executed 0 of 0 ERROR (0.114 secs / 0 secs)
[...]

The changes from step 1 and 2 should be included in the patch that will add ES6 transpilation to our Karma tests.

@bajtos
Copy link
Member Author

bajtos commented Feb 6, 2017

Done.

@bajtos bajtos closed this as completed Feb 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants