search

mbalabash / estimo

Evaluates how long a browser will execute your javascript code
MIT License
170 stars 12 forks source link

1.0 doesn’t work on CI #9

Closed ai closed 5 years ago

ai commented 5 years ago

I very like new features from Estimo 1.0. I already tried to use it in Size Limit https://github.com/ai/size-limit/commit/89b59df42d19fb88a8f440649ba4f5bf97c4a469

Unfortunately, I found an issue. On CI without Chrome estimo freezes (have no result for 20 seconds) https://travis-ci.org/ai/size-limit/jobs/547958183

My suggestions:

  1. We need to test estimo with Chrome and without on Travis CI. You can use matrix to use addons only in one case.
  2. We need to find and fix this bug. Travis CI fixes will help.
mbalabash commented 5 years ago

@ai Yep Without addons -> chrome: stable i have got an error on Travis CI:

No output has been received in the last 10m0s, this potentially indicates a stalled build or something wrong with the build itself.

Travis CI build log

I was trying to fix this yesterday. But i didn't find reason of this error. addons was workaround for this situation.

Seems like tests which runs chrome are not finishing correctly on Travis CI.

ai commented 5 years ago

It will be very hard for me to force every user to add addons: chrome to Travis CI. Also, many other CIs doesn’t have simple addons config.

Maybe @aslushnikov know why find_chrome script can freeze on CIs?

mbalabash commented 5 years ago

@ai find_chrome works as expected. You can see result of this script at line 469

ai commented 5 years ago

@mbalabash but how find_chrome works if there is no chrome (as we have on CI without addon)

mbalabash commented 5 years ago

@ai postinstall npm script starts find_chrome which will download chrome if can't find it. Then it prints chrome location to stdout and save path in file (we can see at line 469).

My guess is something do wrong when we launch puppeteer.

mbalabash commented 5 years ago

@ai Oops No, i'm was wrong. In situation which i described above. Chrome location should be in another place.

mbalabash commented 5 years ago

Update: Default Travis CI image without addons: chrome has local version of google-chrome-stable (L469)

mbalabash commented 5 years ago

@ai Is it possible for you to check Size Limit on another CI?

It might be only Travis CI issue. In docker i also can't reproduce this error.

-- I'm continuing research this situation with Travis CI to find out why this happens.

ai commented 5 years ago

Sorry, in the next few hours I will not be able to test non-Travis CI. Anyway, it is better to add another CIs to your project since your tests are smaller.

mbalabash commented 5 years ago

@ai

Updates: 1) Seems like Travis CI in builds without chrome, anyway adds chrome but without execution privileges. 2) I'm removed from docker file any directives for chrome setup. Then we can test findChrome script in wild environment. 3) Also i fixed bug in findChrome script for linux env. 4) Circle CI and docker throws same error related to chrome: 

{
    message: `Failed to launch chrome!
    /home/circleci/estimo/temp/chrome/linux-669486/chrome-linux/chrome: error while loading shared libraries: libXtst.so.6: cannot open shared object file: No such file or directory
    TROUBLESHOOTING: https://github.com/GoogleChrome/puppeteer/blob/master/docs/troubleshooting.md
    `,
}

5) Travis CI still fails builds by timeout (see 1st point)

Circle CI build log Travis CI build log

mbalabash commented 5 years ago

@aslushnikov Hello

Seems like we need to set up some libs for launching chrome without errors.

Is it possible to use findChrome without any pre installed packages? or install them and all required steps with chrome by puppeteer?

Can you help with this error?

{
    message: `Failed to launch chrome!
    /home/circleci/estimo/temp/chrome/linux-669486/chrome-linux/chrome: error while loading shared libraries: libXtst.so.6: cannot open shared object file: No such file or directory
    TROUBLESHOOTING: https://github.com/GoogleChrome/puppeteer/blob/master/docs/troubleshooting.md
    `,
}
ai commented 5 years ago

@mbalabash let’s just download Chrome on any findChrome issue. We do not need to have 100% sure answer.

ai commented 5 years ago

@mbalabash can we fix the problem if we will download Chrome on findChrome errors?

mbalabash commented 5 years ago

@ai

@mbalabash can we fix the problem if we will download Chrome on findChrome errors?

Oh, i think we can't.

For TravisCI findChrome doesn't throw any error. And i found a lot of issues with Puppeteer on TravisCI. All fixes require changes in .travis.yml For instance: 1 2 3

For CircleCI / docker findChrome works correctly and will install chrome. But chrome can't start without some libs. And we also need to declare those libs in ci config. Common issues: 1 2 3

ai commented 5 years ago

@mbalabash why do we need these changes for .travis.yml now and we didn’t require it for old versions without findChrome?

mbalabash commented 5 years ago

@ai because now we use puppeteer-core and installing chrome when it wasn't located locally.

mbalabash commented 5 years ago

Situation for Travis CI and Docker / Circle CI are different. And for both of them we need some solution. Without it chrome not working correctly (see this).

mbalabash commented 5 years ago

I can switch back to puppeteer while acceptable solution for using puppeteer-core doesn't exist.

Is it have sense for you @ai ?

ai commented 5 years ago

because now we use puppeteer-core and installing chrome when it wasn't located locally.

Previous Estimo < 1.0 installed Chrome too without .travis.yml changes.

Why Chrome installing process in puppeter and puppeter-core is different?

ai commented 5 years ago

Few ideas:

  1. Maybe puppeter-core downloads Chrome in a different way? Do we download it in our findChrome or use it from puppeter-core?
  2. Maybe Puppeter does some special tricks to fix the problem.

We can ask Puppeter’s team or I can send help from Cult of Martians.

(Sorry for bothering you, a lot of users push me for long npm install for Size Limit, but I can’t broke Travis CI in the release :( )

mbalabash commented 5 years ago

@ai

  1. findChrome uses createBrowserFetcher from puppeter-core.
  2. I'm was trying to find out. But now i don't have any solution (

Maybe @aslushnikov can help with this?

(Sorry for delay and slow response speed. Belarus accept new law and now i have some issue with Belarusian army. I'm trying to fix it right now. But now and on next week i can't spend a lot of time for open source 🙁).

Maybe Cult of Martians it's a good idea.

aslushnikov commented 5 years ago

Folks,

If I read the discussion correctly, then manually downloading chrome into Travis is not enough to make it runnable.

This is most likely because default travis env does not include chromium dependencies. I'd speculate that Travis's "chrome" addon installs all of these.

ai commented 5 years ago

@aslushnikov by some reason previous version works on Travis without any changes. Seems like puppeter has fixes missed in puppeter-core. Do you have any ideas how it may work?

aslushnikov commented 5 years ago

Seems like puppeter has fixes missed in puppeter-core

The only difference between puppeteer and puppeteer-core is that Puppeteer runs install.js during its installation.

aslushnikov commented 5 years ago

Ah, also puppeteer respects PUPPETEER_* env variables, whereas puppeteer-core ignores them all.

ai commented 5 years ago

Very strange. Maybe Travis already has some environment variables to fix Chrome?

Or maybe we use different build out different versions in the Puppeter Core and Puppeteer?

ai commented 5 years ago

Made few researchs.

estimo from master and addons: chrome: error estimo from master without addons: chrome: error

Even without addons: chrome findChome.js by some reason find Chrome at /usr/bin/google-chrome-stable.

The same problem in estimo Travis CI: https://travis-ci.org/mbalabash/estimo/jobs/549648463#L470

findChome.js found local Chrome and did not download it.

Seems like we have a problem for Chome detection

mbalabash commented 5 years ago

@ai

This fixed in version estimo@1.1.0 (thanks @creazero !) Security alerts fixed in estimo@1.1.1.