add printing package folder sizes to the build script #5978

bahmutov · 2019-12-16T20:53:29Z

for Trace why the built zip size keeps increasing #5977
zip file is back to 157 MB
also did a little bit of duplicate config code consolidation to prepare for Do not load full environment while building Test Runner #5990
during CI build on Mac and Linux prints the total folder sizes for app/packages/* folders. Example output

3:50:51 PM #printPackageSizes /Users/gleb/git/cypress/build/darwin/Cypress.app/Contents/resources/app/packages darwin
7.8M	/Users/gleb/git/cypress/build/darwin/Cypress.app/Contents/resources/app/packages/https-proxy
 11M	/Users/gleb/git/cypress/build/darwin/Cypress.app/Contents/resources/app/packages/desktop-gui
8.9M	/Users/gleb/git/cypress/build/darwin/Cypress.app/Contents/resources/app/packages/launcher
8.0K	/Users/gleb/git/cypress/build/darwin/Cypress.app/Contents/resources/app/packages/web-config
6.3M	/Users/gleb/git/cypress/build/darwin/Cypress.app/Contents/resources/app/packages/extension
7.7M	/Users/gleb/git/cypress/build/darwin/Cypress.app/Contents/resources/app/packages/proxy
 20K	/Users/gleb/git/cypress/build/darwin/Cypress.app/Contents/resources/app/packages/reporter
 12K	/Users/gleb/git/cypress/build/darwin/Cypress.app/Contents/resources/app/packages/driver
1.1M	/Users/gleb/git/cypress/build/darwin/Cypress.app/Contents/resources/app/packages/example
 10M	/Users/gleb/git/cypress/build/darwin/Cypress.app/Contents/resources/app/packages/runner
 60M	/Users/gleb/git/cypress/build/darwin/Cypress.app/Contents/resources/app/packages/network
360M	/Users/gleb/git/cypress/build/darwin/Cypress.app/Contents/resources/app/packages/server
144K	/Users/gleb/git/cypress/build/darwin/Cypress.app/Contents/resources/app/packages/static
456K	/Users/gleb/git/cypress/build/darwin/Cypress.app/Contents/resources/app/packages/coffee
 12K	/Users/gleb/git/cypress/build/darwin/Cypress.app/Contents/resources/app/packages/root
 27M	/Users/gleb/git/cypress/build/darwin/Cypress.app/Contents/resources/app/packages/electron
136K	/Users/gleb/git/cypress/build/darwin/Cypress.app/Contents/resources/app/packages/ts
3.8M	/Users/gleb/git/cypress/build/darwin/Cypress.app/Contents/resources/app/packages/socket
506M	/Users/gleb/git/cypress/build/darwin/Cypress.app/Contents/resources/app/packages

cypress-bot · 2019-12-16T20:53:32Z

Thanks for the contribution! Below are some guidelines Cypress uses when doing PR reviews.

Please write [WIP] in the title of your Pull Request if your PR is not ready for review - someone will review your PR as soon as the [WIP] is removed.
Please familiarize yourself with the PR Review Checklist and feel free to make updates on your PR based on these guidelines.

PR Review Checklist

If any of the following requirements can't be met, leave a comment in the review selecting 'Request changes', otherwise 'Approve'.

User Experience

The feature/bugfix is self-documenting from within the product.
The change provides the end user with a way to fix their problem (no dead ends).

Functionality

The code works and performs its intended function with the correct logic.
Performance has been factored in (for example, the code cleans up after itself to not cause memory leaks).
The code guards against edge cases and invalid input and has tests to cover it.

Maintainability

The code is readable (too many nested 'if's are a bad sign).
Names used for variables, methods, etc, clearly describe their function.
The code is easy to understood and there are relevant comments explaining.
New algorithms are documented in the code with link(s) to external docs (flowcharts, w3c, chrome, firefox).
There are comments containing link(s) to the addressed issue (in tests and code).

Quality

The change does not reimplement code.
There's not a module from the ecosystem that should be used instead.
There is no redundant or duplicate code.
There are no irrelevant comments left in the code.
Tests are testing the code’s intended functionality in the best way possible.

Internal

The original issue has been tagged with a release in ZenHub.

cypress · 2019-12-16T21:11:01Z

Test summary

3584 • 0 • 47 • 0

Run details


Project	cypress
Status	Passed
Commit	`2979ee7`
Started	Dec 17, 2019 10:26 PM
Ended	Dec 17, 2019 10:31 PM
Duration	04:52 💡
OS	Linux Debian - 9.11
Browser	Multiple

View run in Cypress Dashboard ➡️

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

flotwig · 2019-12-17T18:00:55Z

parse-domain diff for reference: cypress-io/parse-domain@9246927...bahmutov:832822f1c884a464ee309ab695c3784e0bc0980a

flotwig

@bahmutov the changes to parse-domain cause building the PSL to fail:

[driver:install     ] Running sanity check... 
[driver:install     ] Could not update list of known top-level domains for parse-domain because of "Command failed: /home/flotwig/.n/bin/node /tmp/rfff/packages/driver/node_modules/jest/bin/jest.js"
[driver:install     ] Using possibly outdated prebuilt list from Sat Nov 02 2019

It's semi-important that we ship an up-to-date PSL, since Cypress needs to match the PSL behavior of the browser to avoid document.domain errors.

Reviewed in the parse-domain repo: peerigon/parse-domain#91

bahmutov · 2019-12-17T18:14:18Z

So I don't why it says "Nov 2019" in your case. It built the list yesterday when I pointed at my fork - the fact that it tries to run Jest to sanity check this is the 50MB question.

Here is my branch that we are installing https://github.com/bahmutov/parse-domain/tree/dist
It should already include the built files - check https://github.com/bahmutov/parse-domain/blob/dist/build/tries/meta.json

It should have been installed in packages/network too

~/git/cypress/packages/network on print-package-sizes
$ cat node_modules/parse-domain/build/tries/meta.json 
{"updatedAt":"2019-12-16T22:00:57.008Z"}

flotwig · 2019-12-17T18:21:45Z

Lol, that's because the driver is still using parse-domain@2.3.4, not your fork. I guess it's good to know that it's broken even in the official package 🤦‍♂️

I can't tell if it's broken in our CI, since we cache dependencies, the postinstall step doesn't seem to run on every build. Any ideas how to tell?

I do still get an error using the fork in the network package:

[network:install    ] Could not update list of known top-level domains for parse-domain because of "Command failed: /home/flotwig/.n/bin/node /tmp/rfff/node_modules/jest/bin/jest.js"
[network:install    ] Using possibly outdated prebuilt list from Mon Dec 16 2019
[network:install    ] npm WARN sinon-chai@3.3.0 requires a peer of chai@^4.0.0 but none is installed. You must install peer dependencies yourself.
[network:install    ]

It's a little dangerous to rely on a cached PSL stored in a git repo, since it will become stale almost immediately and nobody will check up on it - that is, until we get a bug report of the document.domain issue that occurs.

As a band-aid we could run the build-tries script in the build-binary stage.

bahmutov · 2019-12-17T18:40:15Z

@flotwig I have added postinstall steps to force domain refetches 165978e

packages/driver/package.json

Co-Authored-By: Zach Bloomquist <github@chary.us>

bahmutov · 2019-12-17T21:29:09Z

not really, the official thing is "github:..." but both work.

…

On Tue, Dec 17, 2019 at 4:10 PM Zach Bloomquist ***@***.***> wrote: ***@***.**** approved this pull request. ------------------------------ In packages/driver/package.json <#5978 (comment)>: > @@ -52,7 +54,7 @@ "moment": "2.24.0", "morgan": "1.9.1", "npm-install-version": "6.0.2", - "parse-domain": "2.3.4", + "parse-domain": "github:bahmutov/parse-domain#fb60bd4", never seen this before, we usually do it like this: ⬇️ Suggested change - "parse-domain": "github:bahmutov/parse-domain#fb60bd4", + "parse-domain": "bahmutov/parse-domain#fb60bd4", is there any difference? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#5978?email_source=notifications&email_token=AAQ4BJVIGO3DHW7XDPRUUJDQZE533A5CNFSM4J3QSHLKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCPRAXGY#pullrequestreview-333581211>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAQ4BJSZXUNHXJKRHZW53ODQZE533ANCNFSM4J3QSHLA> .

-- Dr. Gleb Bahmutov, PhD Schedule video chat / phone call / meeting with me via https://calendly.com/bahmutov gleb.bahmutov@gmail.com @bahmutov <https://twitter.com/@bahmutov> https://glebbahmutov.com/ https://glebbahmutov.com/blog https://github.com/bahmutov

…nto print-package-sizes

add printing package folder sizes to the build script

e3f3fc3

bahmutov requested a review from kuceb December 16, 2019 20:53

flotwig previously approved these changes Dec 16, 2019

View reviewed changes

use forked version of parse-domain for now

6872743

bahmutov dismissed flotwig’s stale review via 6872743 December 16, 2019 22:16

bahmutov added 6 commits December 16, 2019 17:40

collect package sizes

160a063

print packages sorted by size

a3c7546

send build package sizes to perf api

5262c73

inject performance tracking context when building linux binary

ffd2565

make sure ci information is not breaking performance send

2bcbbd8

inject the performance context

a71be4c

bahmutov requested a review from flotwig December 17, 2019 15:05

send package sizes as flat object

99921a7

flotwig suggested changes Dec 17, 2019

View reviewed changes

bahmutov added 2 commits December 17, 2019 13:36

use our fork of parse-domain in packages/driver

dcf106d

add parse-domain rebuild as postinstall steps

165978e

parse-domain with better message and build driver

4069e6a

flotwig previously approved these changes Dec 17, 2019

View reviewed changes

packages/driver/package.json Outdated Show resolved Hide resolved

Update packages/driver/package.json

d8fafde

Co-Authored-By: Zach Bloomquist <github@chary.us>

bahmutov dismissed flotwig’s stale review via d8fafde December 17, 2019 21:28

bahmutov added 2 commits December 17, 2019 17:21

consolidate cdn url

9206e09

Merge branch 'print-package-sizes' of github.com:cypress-io/cypress i…

2979ee7

…nto print-package-sizes

flotwig approved these changes Dec 18, 2019

View reviewed changes

bahmutov merged commit 79545fc into develop Dec 18, 2019

emilyrohrbough deleted the print-package-sizes branch August 1, 2024 13:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add printing package folder sizes to the build script #5978

add printing package folder sizes to the build script #5978

bahmutov commented Dec 16, 2019 •

edited

Loading

cypress-bot bot commented Dec 16, 2019

cypress bot commented Dec 16, 2019 •

edited

Loading

flotwig commented Dec 17, 2019

flotwig left a comment

bahmutov commented Dec 17, 2019 •

edited

Loading

flotwig commented Dec 17, 2019 •

edited

Loading

bahmutov commented Dec 17, 2019

bahmutov commented Dec 17, 2019 via email

add printing package folder sizes to the build script #5978

add printing package folder sizes to the build script #5978

Conversation

bahmutov commented Dec 16, 2019 • edited Loading

cypress-bot bot commented Dec 16, 2019

PR Review Checklist

User Experience

Functionality

Maintainability

Quality

Internal

cypress bot commented Dec 16, 2019 • edited Loading

Test summary

Run details

flotwig commented Dec 17, 2019

flotwig left a comment

Choose a reason for hiding this comment

bahmutov commented Dec 17, 2019 • edited Loading

flotwig commented Dec 17, 2019 • edited Loading

bahmutov commented Dec 17, 2019

bahmutov commented Dec 17, 2019 via email

bahmutov commented Dec 16, 2019 •

edited

Loading

cypress bot commented Dec 16, 2019 •

edited

Loading

bahmutov commented Dec 17, 2019 •

edited

Loading

flotwig commented Dec 17, 2019 •

edited

Loading