Skip to content

Update to Node v14#9514

Merged
kumavis merged 2 commits intodevelopfrom
node-v12
Feb 3, 2021
Merged

Update to Node v14#9514
kumavis merged 2 commits intodevelopfrom
node-v12

Conversation

@kumavis
Copy link
Copy Markdown
Member

@kumavis kumavis commented Oct 8, 2020

image

node changelog

https://nodejs.org/en/blog/release/v11.0.0/
https://nodejs.org/en/blog/release/v12.0.0/
https://nodejs.org/en/blog/release/v13.0.0/
https://nodejs.org/en/blog/release/v14.0.0/

@kumavis
Copy link
Copy Markdown
Member Author

kumavis commented Oct 9, 2020

@kumavis
Copy link
Copy Markdown
Member Author

kumavis commented Oct 9, 2020

just benchmarks failing now 🤔

@kumavis
Copy link
Copy Markdown
Member Author

kumavis commented Oct 15, 2020

@Gudahtt dan said you might be familiar with this benchmark failure?

@kumavis kumavis mentioned this pull request Oct 15, 2020
6 tasks
@Gudahtt
Copy link
Copy Markdown
Member

Gudahtt commented Oct 15, 2020

@kumavis Yep! It's an intermittent failure that I've also seen with the Chrome e2e tests. I had spent some time investigating it, but was unsuccessful in finding an explanation about why it's happening or a solution. IIRC I found reports in the chrome web driver issue tracker where a lot of people had the same issue, and the maintainers were saying it was probably a Chrome bug 🤷.

@Gudahtt
Copy link
Copy Markdown
Member

Gudahtt commented Oct 15, 2020

I tried to re-run it, but now CircleCI is failing 😞 It's claiming we don't have pipelines enabled, but we do...

@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [7587458]
Page Load Metrics (274 ± 19 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint35644774
domContentLoaded2433552733919
load2453562743919
domInteractive2433552723919

@kumavis
Copy link
Copy Markdown
Member Author

kumavis commented Nov 11, 2020

@Gudahtt reran CI and it passed! Will need to get this up to date, but very encouraging 😁✌️

@github-actions
Copy link
Copy Markdown
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@EtDu EtDu force-pushed the node-v12 branch 2 times, most recently from bca98bb to 54149a3 Compare November 18, 2020 06:03
@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [9a3e91d]
Page Load Metrics (452 ± 72 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint3110150199
domContentLoaded28982345015072
load29182445215072
domInteractive28982244915072

@EtDu EtDu force-pushed the node-v12 branch 2 times, most recently from f2ef73f to 0826d27 Compare November 19, 2020 06:14
@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [0826d27]
Page Load Metrics (377 ± 43 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint3110342168
domContentLoaded2686143759043
load2696163779043
domInteractive2686143749043

@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [0826d27]
Page Load Metrics (408 ± 54 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint329942147
domContentLoaded27966840711254
load28167140811254
domInteractive27966840711254

@EtDu EtDu marked this pull request as ready for review November 19, 2020 06:56
@EtDu EtDu requested a review from a team as a code owner November 19, 2020 06:56
@EtDu EtDu requested a review from Gudahtt November 19, 2020 06:56
@kumavis
Copy link
Copy Markdown
Member Author

kumavis commented Nov 19, 2020

we should get this merged first MetaMask/core#287


CHROME_VERSION='79.0.3945.117-1'
CHROME_BINARY="google-chrome-stable_${CHROME_VERSION}_amd64.deb"
CHROME_BINARY_URL="http://dl.google.com/linux/chrome/deb/pool/main/g/google-chrome-stable/${CHROME_BINARY}"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh, so the solution to the intermittent Chrome errors was to install it manually? That is interesting.

IIRC Google doesn't leave this URLs up for long, so we may have to frequently bump this. I recall when I was looking for old Chrome versions, I couldn't find any official sources. Perhaps because Google would prefer everybody update.

Copy link
Copy Markdown
Member Author

@kumavis kumavis Nov 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

originally we had bumped the chrome version and chromedriver version, causing intermittent failures
@EtDu manually pulls the old chrome version and reverted the chromedriver bump
we should update the chrome/driver pair but its not actually related to this PR

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, it's unfortunate that we'll now have to download and install Chrome each time, slowing down CI further. But we already do that for Firefox, so we're not much worse off now.

Someday we'll get around to creating a custom Dockerfile for CI so we can start with the exact build environment we want.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe it's a viable solution for the scope of this PR, but we'll definitely want to create a custom image at some point and figure out why the latest chrome is causing intermittent failures.

@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [10f6324]
Page Load Metrics (368 ± 37 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint3010545199
domContentLoaded2725873677737
load2735913687737
domInteractive2715873667737

Copy link
Copy Markdown
Member Author

@kumavis kumavis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • remove package resolution field for ethereumjs-wallet and update to @metamask/controllers@5.0.0 (breaking change should not affect us)

@kumavis kumavis changed the title Node v12 - round 2 Update to Node v12 Nov 19, 2020
@Gudahtt
Copy link
Copy Markdown
Member

Gudahtt commented Nov 19, 2020

I noticed that sesify and sesify-viz have been fully removed in this PR. Were there incompatibilities between that and Node.js v12?

I guess it makes sense to remove for now either way, since sesify is deprecated. Presumably there will be some sesify-viz-like tool integrated with lavamoat at some point?

@kumavis
Copy link
Copy Markdown
Member Author

kumavis commented Nov 19, 2020

yeah they were tied to node v10 for some reason i dont remember
these will be reintroduced to CI as lavamoat-browserify and lavamoat-viz

@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [8536fb8]
Page Load Metrics (414 ± 77 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint309646199
domContentLoaded23772741216177
load23972841416177
domInteractive23772741216177

Copy link
Copy Markdown
Member

@Gudahtt Gudahtt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the sesify-viz removal, the link in the MetaMask bot comment should be removed as well. That is constructed in development/metamaskbot-build-announce.js (seach for deps-viz)

@EtDu EtDu dismissed a stale review via a7235c5 November 23, 2020 01:56
@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [a7235c5]
Page Load Metrics (447 ± 75 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint309744209
domContentLoaded25170544515575
load25370744715575
domInteractive25170544515575

@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [76d8f27]
Page Load Metrics (413 ± 80 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint308241115
domContentLoaded25769041216680
load25969241316680
domInteractive25769041116680

@EtDu EtDu changed the title Update to Node v12 Update to Node v14 Nov 23, 2020
@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [7fbbdde]
Page Load Metrics (376 ± 67 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint287937115
domContentLoaded24359337414067
load24459537614067
domInteractive24359337414067

@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [971fe92]
Page Load Metrics (446 ± 51 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint32100682512
domContentLoaded33068044410751
load33268144610751
domInteractive33067944410751

@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [971fe92]
Page Load Metrics (482 ± 55 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint34125712512
domContentLoaded24975248011555
load25075448211555
domInteractive24875248011555

Copy link
Copy Markdown
Member

@Gudahtt Gudahtt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Copy link
Copy Markdown
Member

@Gudahtt Gudahtt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few new conflicts have cropped up that need to be resolved. Also there were a few new CI jobs added (for testing metrics) that need to be updated to use Node v14

@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [7c369a2]
Page Load Metrics (370 ± 46 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint296747157
domContentLoaded2636203689646
load2656233709746
domInteractive2636193679646

Gudahtt
Gudahtt previously approved these changes Dec 4, 2020
Copy link
Copy Markdown
Member

@Gudahtt Gudahtt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@EtDu EtDu added the DO-NOT-MERGE Pull requests that should not be merged label Dec 7, 2020
@kumavis
Copy link
Copy Markdown
Member Author

kumavis commented Feb 3, 2021

did a manual de-conflict since rebase was becoming very painful

@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [2358abe]
Page Load Metrics (828 ± 11 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint5510382115
domContentLoaded7828718262210
load7838738282211
domInteractive7828708262210

Copy link
Copy Markdown
Member

@rekmarks rekmarks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exciting!

@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [c18e088]
Page Load Metrics (532 ± 46 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint45726084
domContentLoaded3276505309646
load3286515329646
domInteractive3276505309646

@kumavis kumavis merged commit b98cef1 into develop Feb 3, 2021
@kumavis kumavis deleted the node-v12 branch February 3, 2021 05:45
@github-actions github-actions bot locked and limited conversation to collaborators Feb 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

DO-NOT-MERGE Pull requests that should not be merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants