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

return the test in storyshots to respect promises. #2218

Merged

Conversation

travisbloom
Copy link
Contributor

What I did

Jest will wait for promises to resolve before evaluating a test if you return one. This change allows developers to do async work in their test fn

How to test

initStoryshots({
    test: ({ story, context }) => {
        // In the component that is rendered, do some async task that updates the UI after initial render
        const storyElement = story.render(context);
        const tree = renderer.create(storyElement, {});
        return new Promise(resolve => {
            setTimeout(() => {
                const json = tree.toJSON();
                expect(json).toMatchSnapshot();
                resolve();
            }, 0);
        });
    },
});

@codecov
Copy link

codecov bot commented Nov 2, 2017

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2218   +/-   ##
=======================================
  Coverage   21.44%   21.44%           
=======================================
  Files         263      263           
  Lines        5801     5801           
  Branches      692      695    +3     
=======================================
  Hits         1244     1244           
+ Misses       4036     4025   -11     
- Partials      521      532   +11
Impacted Files Coverage Δ
addons/storyshots/src/index.js 0% <0%> (ø) ⬆️
app/react/src/server/config/babel.js 0% <0%> (-100%) ⬇️
app/react/src/server/babel_config.js 0% <0%> (-77.42%) ⬇️
lib/ui/src/modules/ui/containers/down_panel.js 23.52% <0%> (ø) ⬆️
lib/codemod/src/transforms/update-addon-info.js 50% <0%> (ø) ⬆️
...codemod/src/transforms/update-organisation-name.js 40.62% <0%> (ø) ⬆️
...s/left_panel/stories_tree/tree_decorators_utils.js 45.23% <0%> (ø) ⬆️
lib/components/src/navigation/menu_link.js 0% <0%> (ø) ⬆️
addons/knobs/src/components/types/Number.js 8.06% <0%> (ø) ⬆️
addons/knobs/src/components/types/Color.js 8.1% <0%> (ø) ⬆️
... and 20 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 56b9560...47eaec6. Read the comment docs.

Copy link
Member

@danielduan danielduan left a comment

Choose a reason for hiding this comment

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

This looks reasonable to me. I'm not all too familiar with storyshots though.

Anyone with second opinions?

@Hypnosphi Hypnosphi requested a review from ndelangen November 3, 2017 00:36
@ndelangen ndelangen added the bug label Nov 3, 2017
@ndelangen
Copy link
Member

That's a great catch @travisbloom !

Fantastic contribution, than you so much 🙇

@danielduan danielduan merged commit d6003f8 into storybookjs:master Nov 3, 2017
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.

4 participants