feat: Allow tests files in /src (plus Label component tests) #10634
Conversation
There was a problem hiding this comment.
a few comments, but nothing really blocking. this seems like a decent approach to follow going forward, but i'd further reiterate my concern that if we can't do a full migration to this new structure quickly, it'll cause a bunch of confusion for contributors trying to add unit tests
| }; | ||
|
|
||
| const bsStyleKnob = { | ||
| export const bsStyleKnob = { |
There was a problem hiding this comment.
maybe we should move this to a fixtures file that the stories and the test can both import from? It's a bit weird to import from a stories file in the test file, and it would also mean you don't need to tell storybook that this isn't a story too
There was a problem hiding this comment.
In this case (exporting the knobs), I agree it's a little weird, but I really like importing the actual stories in tests, so I figured this could come along for the ride. Mounting the story exports helps make sure your tests cover the use cases of the component properly, and means that storybook edits (i.e. when adding new features/variants to a component) might cause test failures, which I think is a good thing.
All that being said, I'll try to find a good way around the weird knob export/exclusion (probably in another PR). It is a little screwy.
| @@ -230,6 +230,7 @@ const config = { | |||
| }, | |||
| { | |||
| test: /\.tsx?$/, | |||
| exclude: [/\.test.tsx?$/], | |||
There was a problem hiding this comment.
Maybe make this exclude .test.jsx too since there's a decent chance we might move files around before converting them to TS?
There was a problem hiding this comment.
Those are also excluded lower down, in the jsx (babel) loader.
Agreed on the concern... I'm trying to find a good way to reduce the confusion I've already encountered. Right now, it does open up another path, but I'm going to sweep through a bunch of components to follow this pattern and move existing tests, so it becomes more prevalent/obvious. |
Codecov Report
@@ Coverage Diff @@
## master #10634 +/- ##
==========================================
- Coverage 64.30% 59.94% -4.36%
==========================================
Files 778 778
Lines 36782 36802 +20
Branches 3482 3487 +5
==========================================
- Hits 23651 22062 -1589
- Misses 13022 14553 +1531
- Partials 109 187 +78
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
* master: (43 commits) feat: Getting fancier with Storybook (apache#10647) fix: dedup groupby in viz.py while preserving order (apache#10633) feat: bump superset-ui for certified tag (apache#10650) feat: setup react page with submenu for datasources listview (apache#10642) feat: add certification to metrics (apache#10630) feat(viz-plugins): add date formatting to pivot-table (apache#10637) fix: controls scroll issue (apache#10644) feat: Allow tests files in /src (plus Label component tests) (apache#10634) fix: remove duplicated params and cache_timeout from list_columns; add viz_type to list_columns (apache#10643) chore: splitting button stories into separate stories (apache#10631) refactor: remove slice level label_colors from dashboard init load (apache#10603) feat: card view bulk select (apache#10607) style: Label styling/storybook touchups (apache#10627) fix: removing unsupported modal sizes (apache#10625) feat(datasource): remove deleted columns and update column type on metadata refresh (apache#10619) improve documentation for country maps (apache#10621) chore: npm audit fix as of 2020-08-15 (apache#10613) feat: dataset REST API for distinct values (apache#10595) chore: bump react-redux to 5.1.2, whittling console noise (apache#10602) fixing console error about bad html attribute (apache#10604) ... # Conflicts: # superset-frontend/src/explore/components/ExploreViewContainer.jsx # superset-frontend/src/views/App.tsx # superset/config.py
…10634) * allow tests in jest confg * sample stories for Label component * passing tests * stories to tsx! * excluding knobs exports from published stories * ts fix * ts fix * Label test to TS * explicitly ignoring test files in webpack bundling * linting stuff * adding comment about test file exclusions
…10634) * allow tests in jest confg * sample stories for Label component * passing tests * stories to tsx! * excluding knobs exports from published stories * ts fix * ts fix * Label test to TS * explicitly ignoring test files in webpack bundling * linting stuff * adding comment about test file exclusions
mistercrunch
added
🏷️ bot
SUMMARY
This allows test files to live alongside the compnent and storybook files... trying to make things easier to find/audit/move/maintain
This also includes some unit tests for Label, which utilize the neighboring storybook stories, as a POC. I think these storybook entries will actually make it easier to write tests, and vice versa.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TEST PLAN
ADDITIONAL INFORMATION