Legit CITGM failure? #20858

Activity

1. BridgeAR

   added

   streamIssues and PRs related to Node.js streams.Issues and PRs related to Node.js streams.

   on May 20, 2018
2. gireeshpunathil commented on May 21, 2018

   

   gireeshpunathil

   on May 21, 2018

   Member

   More actions

   I can debug these failures, but would appreciate if someone who knows CIGTM can tell me what test it is doing - is it just npm install ws and npm test ? or something else / more?

   /cc @nodejs/platform-macos @nodejs/platform-aix
3. Trott commented on May 21, 2018

   

   Trott

   on May 21, 2018

   Member

   More actions

   @gireeshpunathil I think it downloads the tarball for current ws (5.1.1 as of this writing), untars it, goes into the source directory, does npm install (so that it has everything it needs to run tests), and does npm test.

   👍React with 👍1Reacted by Gireesh Punathil
4. gireeshpunathil commented on May 21, 2018

   

   gireeshpunathil

   on May 21, 2018

   Member

   More actions

   this is upto where I was able to reach, I don't think this is the route CIGTM took, so stopping here and see if @lpinca has better insights.

   #npm test

   > ws@5.1.1 test /Users/gireeshpunathil/Desktop/node_modules/ws/rec/package
   > eslint . && nyc --reporter=html --reporter=text mocha test/*.test.js
   
   
   Oops! Something went wrong! :(
   
   ESLint: 4.19.1.
   ESLint couldn't find a configuration file. To set up a configuration file for this project, please run:
   
       eslint --init
   

   #eslint --init

   ? How would you like to configure ESLint? Use a popular style guide
   ? Which style guide do you want to follow? 
     Google 
     Airbnb 
   ❯ Standard 
   #eslint --init
   ? How would you like to configure ESLint? Inspect your JavaScript file(s)
   ? Which file(s), path(s), or glob(s) should be examined? /Users/gireeshpunathil/Desktop/node_modules/ws/
   ? What format do you want your config file to be in? JavaScript
   ? Are you using ECMAScript 6 features? Yes
   ? Are you using ES6 modules? Yes
   ? Where will your code run? Node
   ? Do you use JSX? No
   
   (/Users/gireeshpunathil/Desktop/node_modules/ws/node_modules/mkdirp/bin/cmd.js:13:5) Parsing error: 'return' outside of function
   ...
   
   
5. Trott commented on May 21, 2018

   

   Trott

   on May 21, 2018

   Last edited by Trott

   Member

   More actions

   @gireeshpunathil Sorry! I should have been more specific. You need to download the source tarball, not the release tarball. Otherwise it won't have things like the ESLint config file. Try the tarball at https://github.com/websockets/ws/archive/5.1.1.tar.gz.
6. gireeshpunathil commented on May 21, 2018

   

   gireeshpunathil

   on May 21, 2018

   Member

   More actions

   no problem, thanks - now I get exactly what is caught in the CI console:

     262 passing (4s)
     1 failing
   
     1) PerMessageDeflate
          #compress and #decompress
            doesn't call the callback twice when `maxPayload` is exceeded:
        Error: Timeout of 2000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves.

   will see what happened inside.
7. lpinca commented on May 21, 2018

   

   lpinca

   on May 21, 2018

   Last edited by lpinca

   Member

   More actions

   Yes, that failure is legit on Node.js 10. That test was calling zlib.flush() after calling zlib.close() and that no longer works in Node.js 10, not sure why it was working in older versions.

   I fixed it in websockets/ws@bb9c21c but didn't publish a new version yet.

   👍React with 👍2Reacted by Gireesh Punathil and Ruben Bridgewater
8. gireeshpunathil commented on May 21, 2018

   

   gireeshpunathil

   on May 21, 2018

   Member

   More actions

   thanks @lpinca ! I was breaking my head on why the compress way not throwing error while decompress does etc.

   I hope we can close this out then.
9. apapirovski commented on May 21, 2018

   

   apapirovski

   on May 21, 2018

   Contributor

   More actions

   Sounds like we can close this. Feel free to reopen if I misunderstood something and this still needs attention in Node.js core.
10. apapirovski

    closed this as completedon May 21, 2018