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.

Sign up for GitHub

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

Jump to bottom

Added custom test function for storyshots #1035

Merged
merged 2 commits into from
May 21, 2017
Merged

Added custom test function for storyshots #1035

merged 2 commits into from
May 21, 2017

Conversation

tmeasday
Copy link
Member

@tmeasday tmeasday commented May 15, 2017 •
edited
Loading

Issue: #1034

What I did

Branched off #971, this pulls the default implementation of the test function into a snapshot export, adds a renderOnly export, and allows you to pass your own function in.

How to test

In the test-cra app, you can try the default, passing in renderOnly or your own function that asserts/throws.

@tmeasday tmeasday requested a review from ndelangen May 15, 2017 05:40
@tmeasday tmeasday requested a review from shilman May 15, 2017 05:40
@tmeasday tmeasday mentioned this pull request May 15, 2017
Closed
shilman
shilman approved these changes May 15, 2017
View reviewed changes
Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

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

Ignore my other review on #971. Prefer renderOnly to justRender. 😸

packages/storyshots/src/index.js Outdated
(pkg.devDependencies && pkg.devDependencies['@kadira/react-native-storybook']) ||
(pkg.dependencies && pkg.dependencies['@kadira/react-native-storybook']);

const hasDependency = function(name) {
Copy link
Member

Choose a reason for hiding this comment

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

<3

@shilman
Copy link
Member

shilman commented May 15, 2017

@tmeasday there are merge conflicts

@tmeasday
Copy link
Member Author

Let me rebase

@tmeasday tmeasday force-pushed the configure-snapshot-test branch from 5df15fa to 48ecba6 Compare May 15, 2017 06:51
@codecov
Copy link

codecov bot commented May 15, 2017 •
edited
Loading

Codecov Report

❗ No coverage uploaded for pull request base (master@4026bd6). Click here to learn what that means.
The diff coverage is 0%.

Impacted file tree graph

@@           Coverage Diff            @@
##             master   #1035   +/-   ##
========================================
  Coverage          ?      0%           
========================================
  Files             ?       1           
  Lines             ?       5           
  Branches          ?       1           
========================================
  Hits              ?       0           
  Misses            ?       4           
  Partials          ?       1
Impacted Files Coverage Δ
packages/storyshots/src/test-bodies.js 0% <0%> (ø)

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 4026bd6...b00d4a4. Read the comment docs.

@tmeasday tmeasday force-pushed the configure-snapshot-test branch from 48ecba6 to 6c971f2 Compare May 15, 2017 07:03
@tmeasday
Copy link
Member Author

LMK if you want me to rebase this onto #1031

danielduan
danielduan reviewed May 20, 2017
View reviewed changes
examples/test-cra/src/storyshots.test.js Outdated

initStoryshots();

// initStoryshots({ test: renderOnly });
Copy link
Member

Choose a reason for hiding this comment

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

If this is an example of something that could be done for user's reference, maybe leave a comment? Otherwise, just remove it.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's basically a way of testing renderOnly (until we figure out a better testing strategy). I'll try to make it better.

Copy link
Member Author

Choose a reason for hiding this comment

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

I decided to just get rid of it for now.

@tmeasday tmeasday force-pushed the configure-snapshot-test branch 2 times, most recently from 838ef89 to 5b2830f Compare May 21, 2017 10:55
@tmeasday tmeasday force-pushed the configure-snapshot-test branch from 5b2830f to b00d4a4 Compare May 21, 2017 11:10
@tmeasday tmeasday merged commit dd8e861 into master May 21, 2017
@shilman shilman added the misc label May 27, 2017
@ndelangen ndelangen changed the title Configure snapshot test Add custom test function for storyshots May 27, 2017
@shilman shilman changed the title Add custom test function for storyshots Added custom test function for storyshots May 28, 2017
@Hypnosphi Hypnosphi deleted the configure-snapshot-test branch August 17, 2017 22:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@danielduan danielduan danielduan left review comments

@shilman shilman shilman approved these changes

@ndelangen ndelangen Awaiting requested review from ndelangen

Assignees
No one assigned
Labels
addon: storyshots feature request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@tmeasday @shilman @danielduan @ndelangen