Errors when running tests locally

[umaar]

Hey I'd like to get Lighthouse tests passing locally so I can make changes and be confident I've not broken anything, but getting a few errors whereas the Travis build seems to be passing. Wondering if I'm missing something?

yarn && yarn build-all ✅️

yarn bundlesize ✅️
yarn diff:sample-json ✅️
yarn lint ✅️

yarn unit ❗️

yarn type-check ✅️⚠️

FYI originally failed with this. But after clearing node modules it started passing.

yarn smoke ✅️

yarn test-clients ❗️

yarn test-viewer ✅️
yarn test-lantern ✅️
yarn test-legacy-javascript ✅️
yarn test-bundle ✅️
yarn i18n:checks✅️
yarn dogfood-lhci✅️

yarn test-docs ✅️⚠️

Update: brew install jq fixed this

Environment Information

• Yarn: 1.22.4
• Node.js version: v13.12.0
• Operating System: macOS Catalina

[patrickhulce]

Thanks @umaar we'd welcome your contributions! :)

re: unit failure

what's the first i18n failure? they're probably related to the intl polyfill if you have node compiled with some icu support already

re: typecheck failure

I experience this on a lot of typescript projects for unknown reasons, clearing node_modules is exactly what I do to fix it too 🤷

re: test-clients failure

it looks like it couldn't even create the puppeteer page at all, do you have CHROME_PATH env variable set to something that isn't compatible with our version of puppeteer?

re: test-docs failure

expected and covered in the readme

Some of our docs have tests that run only in CI by default. If you end up needing to modify our documentation, you'll need to run yarn test-docs locally to make sure they pass.
Additional Dependencies
brew install jq

[brendankenny]

yarn unit-core

That's a toLocaleString() change that I've noticed locally with Node 13 as well, but I haven't looked into what's going on there. There's been some big ICU changes with Node 13, which may be the root cause (if not helpful for finding a fix).

yarn type-check
FYI originally failed with this. But after clearing node modules it started passing

we upgraded to typescript@3.8.3 from 3.5.3 in #10461, so I'd guess getting the updated dep was the issue.

yarn test-docs
Update: brew install jq fixed this

:/

maybe this should at least check for jq and print a better error message if not found

[umaar]

Thanks both, appreciate the help.

• yarn test-clients is now passing! 🤷‍♂️(but no I hadn't set CHROME_PATH to anything)

• yarn unit: The first failure is i18n › Message values are properly formatted › formats correctly with NaN and Infinity numeric values and the only other failure is swap-locale › can change golden LHR english strings into spanish - I downloaded node direct from their downloads page

  • There's been some big ICU changes with Node 13, which may be the root cause

    • Good to know!

[paulirish]

I've gotten errors on i18n items like that before.

yarn unit-core

That's a toLocaleString() change that I've noticed locally with Node 13 as well, but I haven't looked into what's going on there. There's been some big ICU changes with Node 13, which may be the root cause (if not helpful for finding a fix).

Ive been getting this one as well for several weeks. I, too, have node 13 installed.

...okay, did some testing..

i had thought it may be that 13 was using small-icu and we previously had full-icu.. but i don't think that anymore.

now it just seems that the CLDR data for thousands separator for spanish... CHANGED.
note that Chrome also agrees with node 13:

also.. spotchecking a few other locales that do number formatting different from en.. (like nl-BE, de-DE, hu-HU)... in node 12 they all show equivalent formatting to en, but not in node 13 (or in chrome).

huh.

so who knows why spanish was showing unique formatting in node 12 even if it says its an unsupported locale. WEIRD

---

this points to the Intl.js polyfill just being straight up wrong.
it was last published 4 years ago, so that seems reasonable.

I tested out https://www.npmjs.com/package/@formatjs/intl-unified-numberformat briefly and it gives the correct results in 'es' for node 13 (after loading in the locale data).

tbh adopting that right now seems kinda dumb.

i think we may just want to disable this test until we retire support for node 12.

[brendankenny]

I believe most of these have been fixed or are unfortunate flakes. We should fix the flakes, but sometimes it's difficult to figure out a root cause (sometimes you really just need to rm -r node_modules/ && yarn).

Thanks for this issue @umaar! Feel free to file new lists of annoyances for us to fix :)