Build Test feedback #1861

drwpow · 2020-12-07T23:30:13Z

Changes

Followup to comments in #1828. Broken out into a separate PR post-merge to limit disruption in making test suite changes.

Testing

Tests should pass

Docs

No documentation; all testing changes

vercel · 2020-12-07T23:30:15Z

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/pikapkg/snowpack/nscc3bhq9
✅ Preview: https://snowpack-git-drwpow-test-feedback.pikapkg.vercel.app

drwpow · 2020-12-07T23:54:35Z

test/build/base-url-homepage/base-url-homepage.test.js

-  beforeAll(() => {
-    setupBuildTest(__dirname);
+  beforeAll(async () => {
+    await setupBuildTest(__dirname);


setupBuildTest() is now async…

drwpow · 2020-12-07T23:54:58Z

test/build/base-url-homepage/base-url-homepage.test.js


-    files = readFiles(['index.html'], {cwd});
+    files = await readFiles(cwd);


…as is readFiles() (and it was changed to take an entire directory, and read UTF-8-friendly files)

Unfortunately, I had to revert the “async” setup. Though it was working locally, it was consistently failing in CI.

I fought with it for a couple hours, trying to figure out what the issue is. I never got to the bottom of it, but I suspect async weirdness in Jest. Reason is that beforeAll(async() was never originally supported; you’ll see older issues demanding this feature. Then when it was added, I’m seeing very weird behavior that I can’t really explain other than guessing that beforeAll(async() doesn’t use proper Node.js promises and tries to use some weird implementation that works Most Of The Time™ but was failing for us. I can’t explain otherwise why it just would never truly wait for async functions to finish (I tried everything I could think of 😅)

But that said, I don’t think that having synchronous setup functions is necessarily slower. Jest spawns each individual test in its own child process on its own thread, so already this work is being parallelized. If my suspicions are correct, then I think the sync processes here are happening within the context of a child process, so it’s already optimized.

But also, I’m scared with this much difficulty of me or someone else fighting with beforeAll(async() weirdness, so I’d rather just avoid it.

drwpow · 2020-12-07T23:55:59Z

test/test-utils.js

+/** strip whitespace */
+function stripWS(code) {
+  return code.replace(STRIP_WHITESPACE, '');
+}


Old utils from test/build/build.test.js were added here (these were the only functions, but if we need any other inline logic we could always add that here too, as we need it)

drwpow · 2020-12-07T23:57:02Z

test/test-utils.js

-function readFiles(files, {cwd}) {
-  if (!cwd) throw new Error(`cwd option missing, ex: readFiles(files, { cwd: __dirname })`);
+/** take a folder of files, return contents of files (won’t read entire directory for performance) */
+async function readFiles(directory, {ignore} = {}) {


As stated above, this now just reads all UTF-8-friendly files in a directory. But if you have a test file where a particular file is too large, you can always ignore it from glob().

In other words, this assumes test directories are tiny by default and assume it’s OK to read the entire build directory (we were doing that anyway in build.test.js before), but if that assumption isn’t correct you can always ignore individual files from being read into memory (which it should be noted that nothing currently broke build.test.js which was doing this).

FredKSchott · 2020-12-08T02:47:09Z

test/create-snowpack-app/__snapshots__/create-snowpack-app.test.js.snap

@@ -7381,7 +7381,7 @@ exports[`create-snowpack-app app-template-minimal > build: package.json 1`] = `
    \\"test\\": \\"echo \\\\\\"This template does not include a test runner by default.\\\\\\" && exit 1\\"
  },
  \\"devDependencies\\": {
-    \\"snowpack\\": \\"^2.18.1\\"


whoops forgot to regenerate these after publishing a couple of minutes ago, just updated snapshots on main to fix this.

FredKSchott · 2020-12-08T05:23:53Z

man, jest is terrible. Thanks for digging into that, lets hold off for now and revisit in the future. Maybe a good chance to try out uvu

FredKSchott approved these changes Dec 7, 2020

View reviewed changes

drwpow force-pushed the drwpow/test-feedback branch from 79f483f to 6cbb132 Compare December 7, 2020 23:44

vercel bot deployed to Preview December 7, 2020 23:44 View deployment

drwpow force-pushed the drwpow/test-feedback branch from 6cbb132 to 447b170 Compare December 7, 2020 23:52

vercel bot deployed to Preview December 7, 2020 23:52 View deployment

drwpow commented Dec 7, 2020

View reviewed changes

drwpow force-pushed the drwpow/test-feedback branch from 447b170 to 03291cf Compare December 8, 2020 00:09

vercel bot deployed to Preview December 8, 2020 00:09 View deployment

drwpow force-pushed the drwpow/test-feedback branch from 03291cf to d40f0fd Compare December 8, 2020 00:42

vercel bot deployed to Preview December 8, 2020 00:42 View deployment

drwpow force-pushed the drwpow/test-feedback branch from d40f0fd to ceec16f Compare December 8, 2020 00:50

vercel bot deployed to Preview December 8, 2020 00:50 View deployment

drwpow force-pushed the drwpow/test-feedback branch from ceec16f to 76170fd Compare December 8, 2020 00:57

vercel bot deployed to Preview December 8, 2020 00:57 View deployment

drwpow force-pushed the drwpow/test-feedback branch from 76170fd to 84b3360 Compare December 8, 2020 00:59

vercel bot deployed to Preview December 8, 2020 00:59 View deployment

drwpow force-pushed the drwpow/test-feedback branch from 84b3360 to 8fe2b0e Compare December 8, 2020 01:00

vercel bot deployed to Preview December 8, 2020 01:00 View deployment

drwpow force-pushed the drwpow/test-feedback branch from 8fe2b0e to 1b77ae7 Compare December 8, 2020 01:12

vercel bot deployed to Preview December 8, 2020 01:12 View deployment

drwpow force-pushed the drwpow/test-feedback branch from 1b77ae7 to c462b61 Compare December 8, 2020 01:12

vercel bot deployed to Preview December 8, 2020 01:12 View deployment

drwpow force-pushed the drwpow/test-feedback branch from c462b61 to 0793ee6 Compare December 8, 2020 01:13

vercel bot deployed to Preview December 8, 2020 01:13 View deployment

drwpow force-pushed the drwpow/test-feedback branch from 0793ee6 to 39b8c40 Compare December 8, 2020 01:20

vercel bot deployed to Preview December 8, 2020 01:20 View deployment

drwpow force-pushed the drwpow/test-feedback branch from 39b8c40 to a15baa9 Compare December 8, 2020 01:22

vercel bot deployed to Preview December 8, 2020 01:22 View deployment

drwpow force-pushed the drwpow/test-feedback branch from a15baa9 to ad9446d Compare December 8, 2020 01:29

vercel bot deployed to Preview December 8, 2020 01:29 View deployment

drwpow force-pushed the drwpow/test-feedback branch from ad9446d to 0d260f1 Compare December 8, 2020 01:40

vercel bot deployed to Preview December 8, 2020 01:40 View deployment

drwpow force-pushed the drwpow/test-feedback branch from 0d260f1 to 6f6bdbd Compare December 8, 2020 01:50

vercel bot deployed to Preview December 8, 2020 01:50 View deployment

drwpow force-pushed the drwpow/test-feedback branch from 6f6bdbd to b8031bf Compare December 8, 2020 02:03

vercel bot deployed to Preview December 8, 2020 02:03 View deployment

drwpow force-pushed the drwpow/test-feedback branch from b8031bf to 38791cc Compare December 8, 2020 02:11

vercel bot deployed to Preview December 8, 2020 02:11 View deployment

drwpow force-pushed the drwpow/test-feedback branch from 38791cc to 0559902 Compare December 8, 2020 02:15

vercel bot deployed to Preview December 8, 2020 02:15 View deployment

drwpow force-pushed the drwpow/test-feedback branch from 0559902 to 2cba4c6 Compare December 8, 2020 02:22

vercel bot deployed to Preview December 8, 2020 02:22 View deployment

drwpow force-pushed the drwpow/test-feedback branch from 2cba4c6 to 4e03262 Compare December 8, 2020 02:28

vercel bot deployed to Preview December 8, 2020 02:29 View deployment

drwpow force-pushed the drwpow/test-feedback branch from 4e03262 to 0671b80 Compare December 8, 2020 02:41

vercel bot deployed to Preview December 8, 2020 02:41 View deployment

FredKSchott reviewed Dec 8, 2020

View reviewed changes

drwpow force-pushed the drwpow/test-feedback branch from 0671b80 to cd94242 Compare December 8, 2020 03:01

vercel bot deployed to Preview December 8, 2020 03:01 View deployment

Test feedback

bda08af

drwpow force-pushed the drwpow/test-feedback branch from cd94242 to bda08af Compare December 8, 2020 03:04

vercel bot deployed to Preview December 8, 2020 03:04 View deployment

drwpow merged commit 1328d28 into main Dec 8, 2020

drwpow deleted the drwpow/test-feedback branch December 8, 2020 03:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Build Test feedback #1861

Build Test feedback #1861

drwpow commented Dec 7, 2020 •

edited

Loading

vercel bot commented Dec 7, 2020 •

edited

Loading

drwpow Dec 7, 2020

drwpow Dec 7, 2020

drwpow Dec 8, 2020 •

edited

Loading

drwpow Dec 7, 2020

drwpow Dec 7, 2020 •

edited

Loading

FredKSchott Dec 8, 2020

FredKSchott commented Dec 8, 2020


		files = readFiles(['index.html'], {cwd});
		files = await readFiles(cwd);

Build Test feedback #1861

Build Test feedback #1861

Conversation

drwpow commented Dec 7, 2020 • edited Loading

Changes

Testing

Docs

vercel bot commented Dec 7, 2020 • edited Loading

drwpow Dec 7, 2020

Choose a reason for hiding this comment

drwpow Dec 7, 2020

Choose a reason for hiding this comment

drwpow Dec 8, 2020 • edited Loading

Choose a reason for hiding this comment

drwpow Dec 7, 2020

Choose a reason for hiding this comment

drwpow Dec 7, 2020 • edited Loading

Choose a reason for hiding this comment

FredKSchott Dec 8, 2020

Choose a reason for hiding this comment

FredKSchott commented Dec 8, 2020

drwpow commented Dec 7, 2020 •

edited

Loading

vercel bot commented Dec 7, 2020 •

edited

Loading

drwpow Dec 8, 2020 •

edited

Loading

drwpow Dec 7, 2020 •

edited

Loading