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

Initial Karma Support #3381

Merged
merged 65 commits into from Mar 1, 2016
Merged

Initial Karma Support #3381

merged 65 commits into from Mar 1, 2016

Conversation

ggetz
Copy link
Contributor

@ggetz ggetz commented Dec 29, 2015

Added Karma support as a first step for integrating tests as part of Travis-CI build process.

  • Added Karma and necessary karma plugins to package.json, including launchers for:
    • Chrome
    • Firefox
    • IE
    • Safari
  • Configured Karma to run jasmine tests
  • Added test and test-all targets to gulpfile.js
    • test runs karma tests using browser specified in config file, currently Chrome
    • test-all runs karma tests using all browsers available

TODO

  • Add back in minified release and webGL validation support.
  • Update wiki for new test targets (categories and the two above)

Post merge
See #3571

ggetz and others added 28 commits December 17, 2015 15:29
Also some minor cleanup.
@pjcozzi
Copy link
Contributor

pjcozzi commented Jan 1, 2016

Exciting! Can you please update any of the related Contributor's Guides in this branch?

@mramato
Copy link
Member

mramato commented Feb 17, 2016

Does it also need to work for the it function?

It looks like the original code just uses defineSuite and describe, so I think we're good.

@ggetz
Copy link
Contributor Author

ggetz commented Feb 17, 2016

@mramato Also I didn't mean to do that merge, can I undo it somehow?

@ggetz
Copy link
Contributor Author

ggetz commented Feb 17, 2016

It looks like the original code just uses defineSuite and describe, so I think we're good.

OK

@mramato
Copy link
Member

mramato commented Feb 17, 2016

I'll stop by.

@mramato
Copy link
Member

mramato commented Feb 17, 2016

This is almost ready, I'm probably going to make some final tweaks (hopefully tonight) to better fit normal cesium dev workflow and then I'll merge. Thanks @ggetz.

@ggetz
Copy link
Contributor Author

ggetz commented Feb 17, 2016

Awesome, thanks @mramato

@mramato
Copy link
Member

mramato commented Mar 1, 2016

This PR just got more awesome. Building on @ggetz great work, I added Electron as a launcher option and set it to run headless. I then make it the default when running via WebStorm's own Karma runner. This means you can now run the tests from WebStorm with no additional windows or pop-ups and WebStorm presents the test in an easy to read tree with per-test output logs and clickable callstacks when things go wrong.

@pjcozzi and @ggetz would you might syncing this branch and giving it one last try. After running npm install and opening WebStorm, you'll have a Run tests option in the upper right:

image

Give it a try and let me know what you think.

@pjcozzi
Copy link
Contributor

pjcozzi commented Mar 1, 2016

Works great. fit is also great. Is there anything like that for an entire suite we can add for our defineSuite since we (at least, me) often run tests for a suite at a time?

@pjcozzi
Copy link
Contributor

pjcozzi commented Mar 1, 2016

Also, add this to the test guide if you didn't already.

@mramato
Copy link
Member

mramato commented Mar 1, 2016

defineSuite does not exist in Jasmine, I'm not sure of the history as to why Cesium has it's own thing here instead of just using Jasmine describe directly (it's probably worth looking into). That being said, adding fdefineSuite should be easy.

@ggetz
Copy link
Contributor Author

ggetz commented Mar 1, 2016

This works great! Very cool!

Also add `--browsers` flag for running against specific browsers.
@pjcozzi
Copy link
Contributor

pjcozzi commented Mar 1, 2016

I think the defineSuite history is AMD-related, but you would know better than me.

@mramato
Copy link
Member

mramato commented Mar 1, 2016

Okay, fdefineSuite is in and I updated the guides to include both new options as well as a quick Webstorm testing guide.

I also added a --browsers option for specifying browsers on the command line, i.e. npm run test-webgl --browsers Firefox,Safari will run web-gl only tests against Firefox and Safari.

@mramato
Copy link
Member

mramato commented Mar 1, 2016

While I'm sure there are plenty of kinks to work out, I think this is a huge step up for our dev workflow and is ready for master. Since I did a bunch of work with @ggetz, I probably shouldn't be the one to hit merge; so @pjcozzi please do the honors when you are ready.

@pjcozzi
Copy link
Contributor

pjcozzi commented Mar 1, 2016

👍

pjcozzi added a commit that referenced this pull request Mar 1, 2016
@pjcozzi pjcozzi merged commit e342a39 into CesiumGS:master Mar 1, 2016
@mramato
Copy link
Member

mramato commented Mar 1, 2016

@kring you are no longer a liar. You can thank @ggetz for it.

Thanks again @ggetz

@ggetz
Copy link
Contributor Author

ggetz commented Mar 1, 2016

Thanks @mramato !

@kring
Copy link
Member

kring commented Mar 1, 2016

Yay! Thanks @ggetz! :)

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

4 participants