Conversation
marclove
commented
Jan 10, 2017
- Add web-component-tester
- Add browserify for WCT tests
- Add runners for WCT-related tasks
- Add istanbul
- Add codacy & reporting task
c231de5
to
c36b3f1
Compare
Closes #24. |
@marclove is this ready for code review? |
OK I'll review it as soon as I'm back from lunch. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How can I verify that code coverage is being sent correctly?
lib | ||
/bower_components |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to use web-component-tester and webcomponentsjs from npm instead of bower? I'd like to avoid adding two package managers if possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not possible. I tried, but they explicitly want you to use both.
@@ -0,0 +1,77 @@ | |||
<!doctype html> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think pure javascript should be tested in WCT or just web components?
cc @nfiniteset
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My aim was for all specs to be moved to WCT, including pure javascript, React, and Angular specs. That way, we'll only have two test suites:
- WCT for all javascript/component testing
- Gemini + Storybook for visual regression tests
Once WCT launches, it's no slower than any other test runner at running pure javascript specs. Having a third test runner for pure javascript tests would slow tests and add overhead.
The React spec still lives in the old test runner/suite. I planned to move that over as a part of my next story. But I can move them over as a part of this PR if you'd like.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a nice idea to remove a test runner, however we have some tests that are testing code that isn't meant for the browser. E.g. the script tests. I think we'll always need to have mocha for those.
My preference is to keep mocha for stand-alone unit tests and use WCT only for component integration tests against a real browser. I'm happy to have a debate and get into all the details if you feel strongly about this position.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@marclove FYI we briefly discussed this at standup. I think we should merge this in without the ButtonState tests and see how it goes with the Select Menu divided between two test suites. If we decide it's overkill we can remove mocha for browser modules. What do you think?
*/ | ||
|
||
const _components = require('../packages/components/lib/2016-12-01/index'); | ||
window.Button = _components.Button; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this being used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will remove. Artifact of old work.
Overall great job, couple minor questions. |
- Added web-component-tester - Added browserify for WCT tests - Added runners for WCT-related tasks
- Downgrade WCT - Add istanbul - Add codacy & reporting task - Exclude coverage folder from appropriate checks
c36b3f1
to
cfceda8
Compare
@marclove How can I verify that code coverage is being sent correctly? |