[infra] Pin node version for windows-tools test by augustjk · Pull Request #3011 · lit/lit

augustjk · 2022-06-09T23:27:50Z

The latest patch version of node apparently ships with a version of npm that causes our windows-tools test to fail. (It logs deprecation warnings with every npm script run that messes up our cli test assertions). The latest npm version fixes that. npm/cli#4980

However, updating npm for Windows isn't as simple as npm i -g npm@latest https://docs.npmjs.com/try-the-latest-stable-version-of-npm#upgrading-on-windows

It seems easier to just pin the version of node for the windows-tools test to one that shipped with working npm and unpin later once the latest v16 of node includes that.

changeset-bot · 2022-06-09T23:27:52Z

⚠️ No Changeset found

Latest commit: 067a911

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

github-actions · 2022-06-09T23:28:08Z

📊 Tachometer Benchmark Results

Summary

nop-update

lit-html-kitchen-sink: unsure 🔍 -2% - +2% (-0.63ms - +0.73ms)
^{this-change vs tip-of-tree}

render

lit-element-list: unsure 🔍 -1% - +3% (-0.81ms - +2.64ms)
^{this-change vs tip-of-tree}
lit-html-kitchen-sink: unsure 🔍 -3% - +0% (-1.09ms - +0.11ms)
^{this-change vs tip-of-tree}
lit-html-repeat: unsure 🔍 -4% - +3% (-0.49ms - +0.37ms)
^{this-change vs tip-of-tree}
lit-html-template-heavy: unsure 🔍 -2% - +2% (-1.48ms - +1.17ms)
^{this-change vs tip-of-tree}
reactive-element-list: unsure 🔍 -20% - +4% (-19.98ms - +4.13ms)
^{this-change vs tip-of-tree}

update

lit-element-list: unsure 🔍 -3% - +6% (-27.35ms - +54.14ms)
^{this-change vs tip-of-tree}
lit-html-kitchen-sink: unsure 🔍 -2% - +3% (-2.10ms - +3.47ms)
^{this-change vs tip-of-tree}
lit-html-repeat: unsure 🔍 -2% - +4% (-11.49ms - +19.49ms)
^{this-change vs tip-of-tree}
lit-html-template-heavy: unsure 🔍 -1% - +2% (-1.93ms - +2.67ms)
^{this-change vs tip-of-tree}
reactive-element-list: unsure 🔍 -6% - +20% (-71.40ms - +258.17ms)
^{this-change vs tip-of-tree}

update-reflect

lit-element-list: unsure 🔍 -1% - +1% (-10.42ms - +7.82ms)
^{this-change vs tip-of-tree}
reactive-element-list: unsure 🔍 -2% - +1% (-24.44ms - +15.19ms)
^{this-change vs tip-of-tree}

Results

lit-element-list

Browser: chrome-headless 102.0.5005.61
Sample size: 50
Built by: Benchmarks #3337
Commit: 75ba48a

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	90.73ms - 93.57ms	-	unsure 🔍 -1% - +3% -0.81ms - +2.64ms	faster ✔ 19% - 22% 21.45ms - 24.90ms
tip-of-tree tip-of-tree	90.26ms - 92.20ms	unsure 🔍 -3% - +1% -2.64ms - +0.81ms	-	faster ✔ 20% - 22% 22.71ms - 25.48ms
previous-release previous-release	114.34ms - 116.31ms	slower ❌ 23% - 27% 21.45ms - 24.90ms	slower ❌ 25% - 28% 22.71ms - 25.48ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	959.10ms - 1021.06ms	-	unsure 🔍 -3% - +6% -27.35ms - +54.14ms	faster ✔ 31% - 35% 458.36ms - 526.84ms
tip-of-tree tip-of-tree	950.23ms - 1003.15ms	unsure 🔍 -5% - +3% -54.14ms - +27.35ms	-	faster ✔ 32% - 36% 475.78ms - 536.20ms
previous-release previous-release	1468.11ms - 1497.26ms	slower ❌ 45% - 55% 458.36ms - 526.84ms	slower ❌ 47% - 56% 475.78ms - 536.20ms	-

update-reflect

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	956.05ms - 968.01ms	-	unsure 🔍 -1% - +1% -10.42ms - +7.82ms	faster ✔ 7% - 9% 76.53ms - 97.15ms
tip-of-tree tip-of-tree	956.45ms - 970.22ms	unsure 🔍 -1% - +1% -7.82ms - +10.42ms	-	faster ✔ 7% - 9% 74.68ms - 96.39ms
previous-release previous-release	1040.48ms - 1057.26ms	slower ❌ 8% - 10% 76.53ms - 97.15ms	slower ❌ 8% - 10% 74.68ms - 96.39ms	-

lit-html-kitchen-sink

Browser: chrome-headless 102.0.5005.61
Sample size: 30
Built by: Benchmarks #3337
Commit: 75ba48a

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	36.40ms - 37.01ms	-	unsure 🔍 -3% - +0% -1.09ms - +0.11ms	faster ✔ 12% - 16% 5.10ms - 6.74ms
tip-of-tree tip-of-tree	36.68ms - 37.71ms	unsure 🔍 -0% - +3% -0.11ms - +1.09ms	-	faster ✔ 11% - 15% 4.51ms - 6.35ms
previous-release previous-release	41.87ms - 43.38ms	slower ❌ 14% - 18% 5.10ms - 6.74ms	slower ❌ 12% - 17% 4.51ms - 6.35ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	100.53ms - 104.46ms	-	unsure 🔍 -2% - +3% -2.10ms - +3.47ms	unsure 🔍 -5% - +2% -5.14ms - +2.03ms
tip-of-tree tip-of-tree	99.84ms - 103.78ms	unsure 🔍 -3% - +2% -3.47ms - +2.10ms	-	unsure 🔍 -6% - +1% -5.83ms - +1.34ms
previous-release previous-release	101.06ms - 107.05ms	unsure 🔍 -2% - +5% -2.03ms - +5.14ms	unsure 🔍 -1% - +6% -1.34ms - +5.83ms	-

nop-update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	31.47ms - 32.53ms	-	unsure 🔍 -2% - +2% -0.63ms - +0.73ms	faster ✔ 10% - 14% 3.64ms - 5.10ms
tip-of-tree tip-of-tree	31.53ms - 32.37ms	unsure 🔍 -2% - +2% -0.73ms - +0.63ms	-	faster ✔ 10% - 14% 3.77ms - 5.07ms
previous-release previous-release	35.87ms - 36.87ms	slower ❌ 11% - 16% 3.64ms - 5.10ms	slower ❌ 12% - 16% 3.77ms - 5.07ms	-

lit-html-repeat

Browser: chrome-headless 102.0.5005.61
Sample size: 30
Built by: Benchmarks #3337
Commit: 75ba48a

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	13.10ms - 13.71ms	-	unsure 🔍 -4% - +3% -0.49ms - +0.37ms	faster ✔ 6% - 11% 0.93ms - 1.58ms
tip-of-tree tip-of-tree	13.16ms - 13.78ms	unsure 🔍 -3% - +4% -0.37ms - +0.49ms	-	faster ✔ 6% - 10% 0.87ms - 1.52ms
previous-release previous-release	14.55ms - 14.78ms	slower ❌ 7% - 12% 0.93ms - 1.58ms	slower ❌ 6% - 11% 0.87ms - 1.52ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	475.15ms - 491.98ms	-	unsure 🔍 -2% - +4% -11.49ms - +19.49ms	faster ✔ 19% - 25% 115.13ms - 158.52ms
tip-of-tree tip-of-tree	466.56ms - 492.57ms	unsure 🔍 -4% - +2% -19.49ms - +11.49ms	-	faster ✔ 19% - 26% 116.97ms - 164.68ms
previous-release previous-release	600.39ms - 640.39ms	slower ❌ 24% - 33% 115.13ms - 158.52ms	slower ❌ 24% - 35% 116.97ms - 164.68ms	-

lit-html-template-heavy

Browser: chrome-headless 102.0.5005.61
Sample size: 50
Built by: Benchmarks #3337
Commit: 75ba48a

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	66.37ms - 67.99ms	-	unsure 🔍 -2% - +2% -1.48ms - +1.17ms	faster ✔ 15% - 17% 11.54ms - 14.07ms
tip-of-tree tip-of-tree	66.29ms - 68.38ms	unsure 🔍 -2% - +2% -1.17ms - +1.48ms	-	faster ✔ 14% - 17% 11.23ms - 14.08ms
previous-release previous-release	79.02ms - 80.96ms	slower ❌ 17% - 21% 11.54ms - 14.07ms	slower ❌ 16% - 21% 11.23ms - 14.08ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	152.21ms - 155.68ms	-	unsure 🔍 -1% - +2% -1.93ms - +2.67ms	faster ✔ 11% - 14% 20.03ms - 25.46ms
tip-of-tree tip-of-tree	152.06ms - 155.08ms	unsure 🔍 -2% - +1% -2.67ms - +1.93ms	-	faster ✔ 12% - 14% 20.54ms - 25.69ms
previous-release previous-release	174.60ms - 178.78ms	slower ❌ 13% - 17% 20.03ms - 25.46ms	slower ❌ 13% - 17% 20.54ms - 25.69ms	-

reactive-element-list

Browser: chrome-headless 102.0.5005.61
Sample size: 50
Built by: Benchmarks #3337
Commit: 75ba48a

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	79.92ms - 96.60ms	-	unsure 🔍 -20% - +4% -19.98ms - +4.13ms	unsure 🔍 -21% - +3% -20.76ms - +3.29ms
tip-of-tree tip-of-tree	87.49ms - 104.88ms	unsure 🔍 -5% - +23% -4.13ms - +19.98ms	-	unsure 🔍 -13% - +12% -13.09ms - +11.46ms
previous-release previous-release	88.34ms - 105.66ms	unsure 🔍 -4% - +24% -3.29ms - +20.76ms	unsure 🔍 -12% - +14% -11.46ms - +13.09ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	1266.24ms - 1508.62ms	-	unsure 🔍 -6% - +20% -71.40ms - +258.17ms	unsure 🔍 -8% - +18% -99.61ms - +233.70ms
tip-of-tree tip-of-tree	1182.40ms - 1405.70ms	unsure 🔍 -18% - +5% -258.17ms - +71.40ms	-	unsure 🔍 -14% - +10% -186.19ms - +133.52ms
previous-release previous-release	1205.99ms - 1434.78ms	unsure 🔍 -17% - +7% -233.70ms - +99.61ms	unsure 🔍 -10% - +15% -133.52ms - +186.19ms	-

update-reflect

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	1049.47ms - 1077.18ms	-	unsure 🔍 -2% - +1% -24.44ms - +15.19ms	unsure 🔍 -2% - +1% -23.00ms - +14.55ms
tip-of-tree tip-of-tree	1053.79ms - 1082.11ms	unsure 🔍 -1% - +2% -15.19ms - +24.44ms	-	unsure 🔍 -2% - +2% -18.60ms - +19.40ms
previous-release previous-release	1054.89ms - 1080.21ms	unsure 🔍 -1% - +2% -14.55ms - +23.00ms	unsure 🔍 -2% - +2% -19.40ms - +18.60ms	-

_{tachometer-reporter-action v2 for Benchmarks}

AndrewJakubowicz

Nice investigation and fix.

augustjk · 2022-06-10T00:45:52Z

@rictic Do you think there might be a better way? Here's the error that's happening. https://github.com/lit/lit/runs/6822126656?check_suite_focus=true#step:8:1919

The bug is that any npm script run will always spit out

`npm WARN config global `--global`, `--local` are deprecated. Use `--location=global` instead.`

which the test is not expecting to see.

Speaking with @justinfagnani offline, we were wondering if it's possible to pass like a --silent flag to the npm call so that the warning above gets suppressed. Or should the test assertion be written to be able to ignore other outputs?

This is also hopefully just a temporary thing so maybe it'll be fine to pin for now and unpin soon, though at the risk of any future unexpected output resurfacing this again.

justinfagnani

Great fix! I would just file an issue to remember to unpin when we can.

Context: npm/cli#4980, lit/lit#3011

Pin node version for windows-tools test

067a911

augustjk requested review from AndrewJakubowicz and aomarks June 9, 2022 23:27

augustjk requested a review from justinfagnani as a code owner June 9, 2022 23:27

AndrewJakubowicz approved these changes Jun 9, 2022

View reviewed changes

augustjk requested a review from rictic June 10, 2022 00:31

justinfagnani approved these changes Jun 10, 2022

View reviewed changes

augustjk merged commit dd79d44 into main Jun 10, 2022

augustjk deleted the fix-windows-npm branch June 10, 2022 17:10

augustjk mentioned this pull request Jun 10, 2022

[infra] Unpin node version for windows-tools test #3022

Closed

aomarks mentioned this pull request Jun 10, 2022

Temporary CI fix for npm bug google/wireit#290

Merged

aomarks added a commit to google/wireit that referenced this pull request Jun 10, 2022

Temporary CI fix for npm bug (#290)

04fd4d8

Context: npm/cli#4980, lit/lit#3011

Conversation

augustjk commented Jun 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

changeset-bot bot commented Jun 9, 2022

⚠️ No Changeset found

Uh oh!

github-actions bot commented Jun 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📊 Tachometer Benchmark Results

Summary

nop-update

render

update

update-reflect

Results

render

update

update-reflect

render

update

nop-update

render

update

render

update

render

update

update-reflect

Uh oh!

AndrewJakubowicz left a comment

Choose a reason for hiding this comment

Uh oh!

augustjk commented Jun 10, 2022

Uh oh!

justinfagnani left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

augustjk commented Jun 9, 2022 •

edited

Loading

github-actions bot commented Jun 9, 2022 •

edited

Loading