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

Get ES6 unit tests and coverage running #8199

Merged
merged 1 commit into from
Sep 20, 2019
Merged

Get ES6 unit tests and coverage running #8199

merged 1 commit into from
Sep 20, 2019

Conversation

mramato
Copy link
Contributor

@mramato mramato commented Sep 20, 2019

For MVP ES6 support, we're going to run in ES6-aware browsers only, so no IE. Chrome and Firefox works great (and I suspect Safari as well).

We can definitely look into transpiling back to ES5 in a future update, but I'm more worried about overall ES6 support right now.

This loses the ability to run tests against the release build, but I plan on hopefully putting that back in before everything merges up to master.

There are a couple of test failures, but those will get looked into separately than the actual test running mechanism itself.

  1. Runs via node/CI and in the browser (which I honestly didn't think we could keep going).

  2. Switch to karma-coverage-istanbul-instrumenter for instrumentation since we need it for native es6 support

  3. Generate an ES6 version of SpecList.js and update spec-main.js to use it.

  4. Change to karma-conf.js to treat specs as ES6

  5. Fix up some paths/specs to actually work.

Just run the tests as normal to make sure stuff works.

This is targeted to es6-viewer but I'll retarget if that gets merged first.

CC @hpinkos

Unverified

No user is associated with the committer email.
For MVP ES6 support, we're going to run in ES6-aware browsers only, so
no IE.  Chrome and Firefox works great (and I suspect Safari as well).

We can definitely look into transpiling back to ES5 in a future update, but
I'm more worried about overall ES6 support right now.

This loses the ability to run tests against the release build, but I plan
on hopefully putting that back in before everything merges up to master.

There are a couple of test failures, but those will get looked into
separately than the actual test running mechanism itself.

1. Runs via node/CI and in the browser (which I honestly didn't think we
could keep going).

2. Switch to `karma-coverage-istanbul-instrumenter` for instrumentation
since we need it for native es6 support

3. Generate an ES6 version of SpecList.js and update spec-main.js to use
it.

4. Change to karma-conf.js to treat specs as ES6

5. Fix up some paths/specs to actually work.
@cesium-concierge
Copy link

Thanks for the pull request @mramato!

  • ✔️ Signed CLA found.

Reviewers, don't forget to make sure that:

  • Cesium Viewer works.
  • Works in 2D/CV.
  • Works (or fails gracefully) in IE11.

Sorry, something went wrong.

@mramato mramato requested a review from hpinkos September 20, 2019 14:37
@hpinkos
Copy link
Contributor

hpinkos commented Sep 20, 2019

Like you said, some tests are failing, but coverage and running tests in the browser appears to be working well. I don't have any feedback

@mramato
Copy link
Contributor Author

mramato commented Sep 20, 2019

Talking with @hpinkos offline, to move faster we're going to switch to a coarse 2 PR approach. Basically #8192 will stay open until the end (as planned) but everything else will go into #8193 right away. This way you don't have the 16k lines of cruft getting in the way of the review, but reviewers can just do on PR review at the end.

@mramato mramato merged commit 4fa71fd into es6-viewer Sep 20, 2019
@mramato mramato deleted the es6-tests branch September 20, 2019 16:50
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.

None yet

3 participants