From cbd4847d2bbb3770695ba9fd1d31a426f793cb3e Mon Sep 17 00:00:00 2001 From: Ninad Sheth Date: Thu, 12 Oct 2023 14:50:37 +0530 Subject: [PATCH 1/3] Now we skip story on error than crash --- src/snapshots.js | 78 +++++++++++++++++++++++++++++------------------- 1 file changed, 48 insertions(+), 30 deletions(-) diff --git a/src/snapshots.js b/src/snapshots.js index e4108d44..0a087609 100644 --- a/src/snapshots.js +++ b/src/snapshots.js @@ -188,38 +188,56 @@ export async function* takeStorybookSnapshots(percy, callback, { baseUrl, flags // set storybook environment info percy.client.addEnvironmentInfo(environmentInfo); - // use a single page to capture story snapshots without reloading - yield* withPage(percy, previewUrl, async function*(page) { - // determines when to retry page crashes - lastCount = snapshots.length; - - while (snapshots.length) { - // separate story and snapshot options - let { id, args, globals, queryParams, ...options } = snapshots[0]; - - if (flags.dryRun || options.enableJavaScript || percy.config.snapshot.enableJavaScript) { - // when dry-running or when javascript is enabled, use the preview dom - options.domSnapshot = previewResource.content; - } else { - // when not dry-running and javascript is not enabled, capture the story dom - yield page.eval(evalSetCurrentStory, { id, args, globals, queryParams }); - /* istanbul ignore next: tested, but coverage is stripped */ - let { dom, domSnapshot = dom } = yield page.snapshot(options); - options.domSnapshot = domSnapshot; - } - - // validate without logging to prune all other options - PercyConfig.validate(options, '/snapshot/dom'); - // snapshots are queued and do not need to be awaited on - percy.snapshot(options); - // discard this story snapshot when done + while (snapshots.length) { + try { + // use a single page to capture story snapshots without reloading + yield* withPage(percy, previewUrl, async function*(page) { + // determines when to retry page crashes + lastCount = snapshots.length; + + while (snapshots.length) { + // separate story and snapshot options + let { id, args, globals, queryParams, ...options } = snapshots[0]; + + if (flags.dryRun || options.enableJavaScript || percy.config.snapshot.enableJavaScript) { + log.debug(`Loading story via previewResource: ${options.name}`); + // when dry-running or when javascript is enabled, use the preview dom + options.domSnapshot = previewResource.content; + } else { + log.debug(`Loading story: ${options.name}`); + // when not dry-running and javascript is not enabled, capture the story dom + yield page.eval(evalSetCurrentStory, { id, args, globals, queryParams }); + /* istanbul ignore next: tested, but coverage is stripped */ + let { dom, domSnapshot = dom } = yield page.snapshot(options); + options.domSnapshot = domSnapshot; + } + + // validate without logging to prune all other options + PercyConfig.validate(options, '/snapshot/dom'); + // snapshots are queued and do not need to be awaited on + percy.snapshot(options); + // discard this story snapshot when done + snapshots.shift(); + } + }, () => { + log.debug(`Page crashed while loading story: ${snapshots[0].name}`); + // return true to retry as long as the length decreases + return lastCount > snapshots.length; + }); + } catch (e) { + // if we get an exception while capturing a story + // - we want to print error + // - skip story + // - continue capturing stories on a new page to avoid weird page states + let { name } = snapshots[0]; + + log.error(`Failed to capture story: ${name}`); + log.error(e); + + // ignore story snapshots.shift(); } - }, () => { - log.debug(`Page crashed while loading story: ${snapshots[0].name}`); - // return true to retry as long as the length decreases - return lastCount > snapshots.length; - }); + } // will stop once snapshots are done processing yield* percy.yield.stop(); From ed6f949e647b9c13d2f2de0b7c1d9cc126daa790 Mon Sep 17 00:00:00 2001 From: Ninad Sheth Date: Fri, 13 Oct 2023 16:28:54 +0530 Subject: [PATCH 2/3] Added tests --- src/snapshots.js | 20 +++++++++----------- test/storybook.test.js | 31 +++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 11 deletions(-) diff --git a/src/snapshots.js b/src/snapshots.js index 0a087609..97d0adc0 100644 --- a/src/snapshots.js +++ b/src/snapshots.js @@ -225,17 +225,15 @@ export async function* takeStorybookSnapshots(percy, callback, { baseUrl, flags return lastCount > snapshots.length; }); } catch (e) { - // if we get an exception while capturing a story - // - we want to print error - // - skip story - // - continue capturing stories on a new page to avoid weird page states - let { name } = snapshots[0]; - - log.error(`Failed to capture story: ${name}`); - log.error(e); - - // ignore story - snapshots.shift(); + if (process.env.PERCY_SKIP_STORY_ON_ERROR === 'true') { + let { name } = snapshots[0]; + log.error(`Failed to capture story: ${name}`); + log.error(e); + // ignore story + snapshots.shift(); + } else { + throw e; + } } } diff --git a/test/storybook.test.js b/test/storybook.test.js index 51bf551e..da407177 100644 --- a/test/storybook.test.js +++ b/test/storybook.test.js @@ -155,6 +155,37 @@ describe('percy storybook', () => { ]); }); + describe('with PERCY_SKIP_STORY_ON_ERROR set to true', () => { + beforeAll(() => { + process.env.PERCY_SKIP_STORY_ON_ERROR = true; + }); + + afterAll(() => { + delete process.env.PERCY_SKIP_STORY_ON_ERROR; + }); + + it('skips the story and logs the error but does not break build', async () => { + server.reply('/iframe.html', () => [200, 'text/html', [ + ``, + `` + ].join('')]); + + // does not reject + await storybook(['http://localhost:8000']); + + // contains logs of story error + expect(logger.stderr).toEqual([ + '[percy] Failed to capture story: foo: bar', + // error logs contain the client stack trace + jasmine.stringMatching(/^\[percy\] Error: Story Error\n.*\/iframe\.html.*$/s) + ]); + }); + }); + it('uses the preview dom when javascript is enabled', async () => { let previewDOM = '

This is the preview

'; let i = 0; From 25c2b8a257e0ad01e03bfa8b719d9f3474289925 Mon Sep 17 00:00:00 2001 From: Ninad Sheth Date: Fri, 13 Oct 2023 17:00:33 +0530 Subject: [PATCH 3/3] Added a comment and fixed test --- src/snapshots.js | 8 ++++++++ test/storybook.test.js | 4 +++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/snapshots.js b/src/snapshots.js index 97d0adc0..2b56d426 100644 --- a/src/snapshots.js +++ b/src/snapshots.js @@ -188,6 +188,14 @@ export async function* takeStorybookSnapshots(percy, callback, { baseUrl, flags // set storybook environment info percy.client.addEnvironmentInfo(environmentInfo); + // We use an outer and inner loop on same snapshots.length + // - we create a new page and load one story on it at a time for snapshotting + // - if it throws exception then we want to catch it outside of `withPage` call as + // when `withPage` returns it closes the page + // - we want to make sure we close the page that had exception in story to make sure + // we dont reuse a page which is possibly in a weird state due to last exception + // - so post exception we come out of inner loop and skip the story, create new page + // using outer loop and continue next stories again on a new page while (snapshots.length) { try { // use a single page to capture story snapshots without reloading diff --git a/test/storybook.test.js b/test/storybook.test.js index da407177..628bd3ac 100644 --- a/test/storybook.test.js +++ b/test/storybook.test.js @@ -181,7 +181,9 @@ describe('percy storybook', () => { expect(logger.stderr).toEqual([ '[percy] Failed to capture story: foo: bar', // error logs contain the client stack trace - jasmine.stringMatching(/^\[percy\] Error: Story Error\n.*\/iframe\.html.*$/s) + jasmine.stringMatching(/^\[percy\] Error: Story Error\n.*\/iframe\.html.*$/s), + // does not create a build if all stories failed [ 1 in this case ] + '[percy] Build not created' ]); }); });