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

Made jest test file extension name optional #2373

Merged
merged 2 commits into from
Nov 25, 2017

Conversation

promisetochi
Copy link
Contributor

@promisetochi promisetochi commented Nov 25, 2017

#2368:

What I did

Updated empty string to be used as default value for extension name, if it's not passed as an argument

How to test

Is this testable with jest or storyshots? Not sure

Does this need a new example in the kitchen sink apps? No

Does this need an update to the documentation? Not sure

If your answer is yes to any of these, please make sure to include it in your PR.

@codecov
Copy link

codecov bot commented Nov 25, 2017

Codecov Report

Merging #2373 into master will not change coverage.
The diff coverage is 0%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2373   +/-   ##
=======================================
  Coverage   19.92%   19.92%           
=======================================
  Files         293      293           
  Lines        6484     6484           
  Branches      762      755    -7     
=======================================
  Hits         1292     1292           
- Misses       4579     4615   +36     
+ Partials      613      577   -36
Impacted Files Coverage Δ
addons/jest/src/index.js 0% <0%> (ø) ⬆️
lib/ui/src/modules/ui/containers/left_panel.js 25.71% <0%> (ø) ⬆️
addons/storyshots/src/storybook-channel-mock.js 0% <0%> (ø) ⬆️
app/react/src/client/preview/error_display.js 0% <0%> (ø) ⬆️
addons/knobs/src/components/PropField.js 10.63% <0%> (ø) ⬆️
lib/ui/src/modules/ui/routes.js 0% <0%> (ø) ⬆️
addons/info/src/components/PropTable.js 21.36% <0%> (ø) ⬆️
addons/info/src/components/markdown/htags.js 30% <0%> (ø) ⬆️
addons/knobs/src/KnobStore.js 9.09% <0%> (ø) ⬆️
lib/ui/src/modules/ui/components/layout/usplit.js 22.58% <0%> (ø) ⬆️
... and 21 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update df232b3...0803483. Read the comment docs.

Copy link
Member

@Hypnosphi Hypnosphi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Hypnosphi Hypnosphi self-assigned this Nov 25, 2017
@@ -1,6 +1,6 @@
import addons from '@storybook/addons';

const findTestResults = (testFiles, jestTestResults, jestTestFilesExt) =>
const findTestResults = (testFiles, jestTestResults, jestTestFilesExt = "") =>
Copy link
Member

@Hypnosphi Hypnosphi Nov 25, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if you ran git commit --no-verify instead of just git commit? We have a git hook which runs eslint in "fix" mode, which would replace those double quotes with single ones

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likely didn't do yarn install, it's OK, we'll do a fix commit soon.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I'll run yarn lint:js --fix after merging

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yikes, noticed the double quotes after commiting, thanks!

@Hypnosphi Hypnosphi merged commit bec8bce into storybookjs:master Nov 25, 2017
@Hypnosphi
Copy link
Member

Got to revert, sorry for confusion =(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants