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

Show progress of the addons-storyshot run #3590

Closed
markelog opened this issue May 15, 2018 · 18 comments
Closed

Show progress of the addons-storyshot run #3590

markelog opened this issue May 15, 2018 · 18 comments

Comments

@markelog
Copy link
Contributor

markelog commented May 15, 2018

Bug or support request summary

When executing screenshot tests with addons-storyshot plugin on the large project it shows only initial start message then it looks like it froze when in reality it just has a lot things to do. I'm wondering it it's all possible to show the progress of it?

Steps to reproduce

I suppose default setup of addons-storyshot will do

Please specify which version of Storybook and optionally any affected addons that you're running

  • @storybook/react@3.2.12
  • @storybook/addon-storyshots@4.0.0-alpha.6
@Keraito
Copy link
Contributor

Keraito commented May 15, 2018

I tried my hands a little bit onto this, but any attempts in trying to log something to the terminal from https://github.com/storybooks/storybook/blob/master/addons/storyshots/src/index.js when running the tests failed for me. This seem to involve some irregular longstanding issue at Jest regarding console output being swallowed by the tests output. Maybe @ndelangen you have a bit more insight into a way for this?

@ndelangen
Copy link
Member

We should probably reach out the the jest team and ask them if they'd have some way in mind to get this. I know we're already kinda doing things out of the ordinary.

Maybe @orta or @cpojer or @SimenB could help here?

@SimenB
Copy link
Contributor

SimenB commented May 15, 2018

I have no idea how storyshots is implemented under the hood beyond a cursory look now, but I think some way of making separate story files into separate test files would be the best. We collect all results from a single test file before passing the result on to reporters. I don't think we have a way of streaming the results per test.

That would also help keep the .snap files smaller, and utilize parallelization.

Not sure how feasible that is, though.

@ndelangen
Copy link
Member

Hmm I'm guessing a totally alternative route that would make that work, would be to add story-files as test-files themselves and have a transformer applied on them?

But that sounds easier then it is, since storybook support global options and decorators..
Still.. not impossible.

@stale stale bot added the inactive label Jun 7, 2018
@stale stale bot closed this as completed Jul 7, 2018
@SimenB
Copy link
Contributor

SimenB commented Jul 7, 2018

FWIW we've got an issue for reporting tests as completed as they are, rather than wait for the whole suite to complete (jestjs/jest#6616).

I'd still recommend looking into splitting it up into multiple test files for better concurrency.

@Keraito
Copy link
Contributor

Keraito commented Jul 7, 2018

Re-opening this issue based on the previous comment as the related topic is atm being discussed in Jest itself.

I'd still recommend looking into splitting it up into multiple test files for better concurrency.

@ndelangen would it make sense to make a note about this in the storyshots documentation? Or hold of to see what happens at Jest's side?

@Keraito Keraito reopened this Jul 7, 2018
@stale stale bot removed the inactive label Jul 7, 2018
@igor-dv
Copy link
Member

igor-dv commented Jul 12, 2018

BTW, with the new feature, It might be much easier

@ndelangen
Copy link
Member

ndelangen commented Jul 26, 2018

Interesting @igor-dv !

The splitting into files you mean?
Yeah I can see how.

Requires a bulk of work on the user's part that way.

If we could leverage the component-story files themselves, people would not have to add any files, in fact remove some..

@ndelangen
Copy link
Member

On the roadmap:
https://storybook.canny.io/roadmap/p/storyshots-continues-feedback-during-run

@stale stale bot removed the inactive label Sep 7, 2018
@SimenB
Copy link
Contributor

SimenB commented Sep 12, 2018

Nice! I wonder if that'll fix/help #3286 at the same time

@ndelangen
Copy link
Member

Yeah possibly

@stale stale bot added the inactive label Oct 3, 2018
@storybookjs storybookjs deleted a comment from stale bot Oct 12, 2018
@stale stale bot removed the inactive label Oct 12, 2018
@storybookjs storybookjs deleted a comment from stale bot Oct 12, 2018
@storybookjs storybookjs deleted a comment from stale bot Oct 12, 2018
@storybookjs storybookjs deleted a comment from stale bot Oct 12, 2018
@ndelangen ndelangen added the todo label Oct 12, 2018
@verticalgrain
Copy link

verticalgrain commented Sep 17, 2019

Update in 2019, one can do this:

const beforeScreenshot = ( page, { context: { kind, story }, url } ) => {
	console.log( 'Generating screenshot for: ' + story );
	return new Promise( resolve =>
		setTimeout( () => {
			resolve();
		}, 2000 )
	)
}

@SQReder
Copy link

SQReder commented Oct 11, 2019

@verticalgrain I did the same, but without delay

@SimenB
Copy link
Contributor

SimenB commented Aug 17, 2020

If storyshots is already separate test cases within a test file (which I believe it is: https://github.com/storybookjs/storybook/blob/0cae839ac299edcdc5e69116913835a769173f1f/addons/storyshots/storyshots-core/src/api/snapshotsTestsTemplate.ts), this should be fixed in Jest 26.3+ if you use jest-circus (which will be the default in Jest 27).

Again, separate files for tests are probably better in order to properly utilize parallelization across CPUs, but the originally reported "no feedback until complete" should be solved.

@ndelangen
Copy link
Member

@SimenB that's awesome!

related: #4996

@bard
Copy link

bard commented Sep 13, 2020

Again, separate files for tests are probably better in order to properly utilize parallelization across CPUs, but the originally reported "no feedback until complete" should be solved.

@SimenB is there any way to feed Jest "virtual" (i.e. not written to disk) files generated on the fly? That way one could generate one test file per story or one file per story kind. Parallelization aside, I'm interested in the possibility of relating those test files back to their respective story files via import, so in theory Jest's --findRelatedFiles could kick in, and only tests for modified components would run.

@SimenB
Copy link
Contributor

SimenB commented Nov 1, 2020

@bard no, Jest currently only works on a real FS. Not sure if it's worth abstracting out the file system entirely (e.g. lots of Jest's internal modules are used outside of Jest, so the API might get hairy). That said, we've been wanting to virtualize the FS previously to make it possible to run Jest in the browser: jestjs/jest#848. That has stagnated, though

@ndelangen
Copy link
Member

The future of storyshots is the test-runner:
https://storybook.js.org/docs/react/writing-tests/test-runner#page-top

And using the play function for asserts:
https://storybook.js.org/docs/react/writing-stories/play-function#page-top

We will not be making significant improvements/changes to storyshots.

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

No branches or pull requests

8 participants