search

nodejs / citgm

Canary in the Gold Mine
https://www.npmjs.com/package/citgm
Other
569 stars 148 forks source link

tape failing on multiple versions and platforms #895

Open BethGriggs opened 2 years ago

BethGriggs commented 2 years ago

Extracting this from #894 because I have discovered that there are different failures across multiple versions/platforms.

Most platforms fail with this error [Resolved]:

 > tape@5.5.2 prepublish
 > !(type not-in-publish) || not-in-publish || npm run prepublishOnly
 not-in-publish is /home/iojs/tmp/citgm_tmp/bc3d8cd6-beed-4db0-ae20-8394e5f7f0c5/tape/node_modules/.bin/not-in-publish
 added 759 packages in 43s
 > tape@5.5.2 pretest
 > npm run lint
 > tape@5.5.2 prelint
 > eclint check $(git ls-files 2>/dev/null | xargs find 2> /dev/null | grep -vE 'node_modules|\.git' || echo '*.md *.js test/*.js')
 node:events:504
       throw er; // Unhandled 'error' event
       ^
 Error: File not found with singular glob: /home/iojs/tmp/citgm_tmp/bc3d8cd6-beed-4db0-ae20-8394e5f7f0c5/tape/test/has (if this was purposeful, use `allowEmpty` option)
     at Glob.<anonymous> (/home/iojs/tmp/citgm_tmp/bc3d8cd6-beed-4db0-ae20-8394e5f7f0c5/tape/node_modules/glob-stream/readable.js:84:17)
     at Object.onceWrapper (node:events:646:26)
     at Glob.emit (node:events:526:28)
     at Glob._finish (/home/iojs/tmp/citgm_tmp/bc3d8cd6-beed-4db0-ae20-8394e5f7f0c5/tape/node_modules/glob/glob.js:194:8)
     at done (/home/iojs/tmp/citgm_tmp/bc3d8cd6-beed-4db0-ae20-8394e5f7f0c5/tape/node_modules/glob/glob.js:179:14)
     at Glob._processSimple2 (/home/iojs/tmp/citgm_tmp/bc3d8cd6-beed-4db0-ae20-8394e5f7f0c5/tape/node_modules/glob/glob.js:685:12)
     at /home/iojs/tmp/citgm_tmp/bc3d8cd6-beed-4db0-ae20-8394e5f7f0c5/tape/node_modules/glob/glob.js:673:10
     at Glob._stat2 (/home/iojs/tmp/citgm_tmp/bc3d8cd6-beed-4db0-ae20-8394e5f7f0c5/tape/node_modules/glob/glob.js:769:12)
     at lstatcb_ (/home/iojs/tmp/citgm_tmp/bc3d8cd6-beed-4db0-ae20-8394e5f7f0c5/tape/node_modules/glob/glob.js:761:12)
     at RES (/home/iojs/tmp/citgm_tmp/bc3d8cd6-beed-4db0-ae20-8394e5f7f0c5/tape/node_modules/inflight/inflight.js:31:16)
 Emitted 'error' event on DestroyableTransform instance at:
     at Pumpify.emit (node:events:526:28)
     at Pumpify.Duplexify._destroy (/home/iojs/tmp/citgm_tmp/bc3d8cd6-beed-4db0-ae20-8394e5f7f0c5/tape/node_modules/duplexify/index.js:191:15)
     at /home/iojs/tmp/citgm_tmp/bc3d8cd6-beed-4db0-ae20-8394e5f7f0c5/tape/node_modules/duplexify/index.js:182:10
     at processTicksAndRejections (node:internal/process/task_queues:78:11)

Windows bails out earlier with a separate error:

  > tape@5.5.2 prepublish
 > !(type not-in-publish) || not-in-publish || npm run prepublishOnly
 added 759 packages in 34s
 > tape@5.5.2 pretest
 > npm run lint
 > tape@5.5.2 prelint
 > eclint check $(git ls-files 2>/dev/null | xargs find 2> /dev/null | grep -vE 'node_modules|\.git' || echo '*.md *.js test/*.js')
 '!' is not recognized as an internal or external command,
 operable program or batch file.
 The system cannot find the path specified.

Interestingly macOS and AIX seem to consistently pass:

Test run links:

BethGriggs commented 2 years ago

FYI @ljharb - it seems there are other issues in addition to the windows issue surfaced in #894

ljharb commented 2 years ago

Thanks, that's good to know. Luckily they're all issues with the dev-only eclint script, but it's still potentially masking real problems, so I'd love to fix it.

It might be an issue with git ls-files printing out the file test/has spaces.js, which has an unescaped space in it. Perhaps some OS's trip over this, but it works fine on Mac OS (and ubuntu, since that's what's used in github actions).

ljharb commented 2 years ago

If I could get access to those machines, or if anyone can reproduce it on their machine, it'd be great to figure out what $(git ls-files 2>/dev/null | xargs find 2> /dev/null | grep -vE 'node_modules|\.git' || echo '*.md *.js test/*.js') outputs. i may need to double-quote it.

BethGriggs commented 2 years ago

Seems plausible. We do have both ubuntu1804 and ubuntu1604 in the matrix on Jenkins and it's failing in the same way there too. But, that's possibly a difference in our host setup in Jenkins versus GH actions.

Access to the linux machines should be much easier to distribute than Windows ones you requested in https://github.com/nodejs/build/issues/2913. I don't have access myself, and I am not sure how quickly the tmp directories are cleaned up so some manual steps may be needed to recreate.

ljharb commented 2 years ago

Should i file an issue for one of these too?

BethGriggs commented 2 years ago

Sure, or maybe you could rename https://github.com/nodejs/build/issues/2913 to be a generic request and request an additional os/arch within there? I don't personally see the need to gather approvals for your access on a per platform basis, but would defer to @nodejs/build for whatever makes their access management easier.

BethGriggs commented 2 years ago

@ljharb I got a little creative with our Jenkins job and managed to get the output of that command in the output - see https://ci.nodejs.org/job/citgm-smoker-nobuild/nodes=ubuntu1804-64/1159/consoleText

richardlau commented 2 years ago

Thanks, that's good to know. Luckily they're all issues with the dev-only eclint script, but it's still potentially masking real problems, so I'd love to fix it.

It might be an issue with git ls-files printing out the file test/has spaces.js, which has an unescaped space in it. Perhaps some OS's trip over this, but it works fine on Mac OS (and ubuntu, since that's what's used in github actions).

FWIW I don't think git ls-files will print anything -- in fact I expect it to error out. From the CITGM job output, e.g. https://ci.nodejs.org/job/citgm-smoker-nobuild/1150/nodes=ubuntu1804-64/console

17:36:42 info: lookup              | tape                
17:36:42 info: lookup-found        | tape                
17:36:42 info: tape lookup-replace | https://github.com/substack/tape/archive/HEAD.tar.gz
17:36:42 info: tape npm:           | Downloading project: https://github.com/substack/tape/archive/HEAD.tar.gz
17:36:42 info: tape npm:           | Project downloaded HEAD.tar.gz
17:36:42 info: tape npm:           | npm install started 
17:36:42 verbose: tape npm:           | Using temp directory: "/home/iojs/tmp/citgm_tmp/bef89702-9128-4153-9811-33d5cef3002d/npm_config_tmp"
17:37:05 verbose: tape npm-install:   | > tape@5.5.2 prepublish /home/iojs/tmp/citgm_tmp/bef89702-9128-4153-9811-33d5cef3002d/tape
17:37:05 verbose:                     | > !(type not-in-publish) || not-in-publish || npm run prepublishOnly                      
17:37:05 verbose: tape npm-install:   | not-in-publish is /home/iojs/tmp/citgm_tmp/bef89702-9128-4153-9811-33d5cef3002d/tape/node_modules/.bin/not-in-publish
17:37:05 verbose: tape npm-install:   | added 961 packages from 641 contributors in 22.843s
17:37:05 info: tape npm:           | npm install successfully completed
17:37:05 info: tape npm:           | test suite started  
17:37:06 verbose: tape npm-test:      | > tape@5.5.2 pretest /home/iojs/tmp/citgm_tmp/bef89702-9128-4153-9811-33d5cef3002d/tape
17:37:06 verbose:                     | > npm run lint                                                                         
17:37:06 verbose: tape npm-test:      | > tape@5.5.2 prelint /home/iojs/tmp/citgm_tmp/bef89702-9128-4153-9811-33d5cef3002d/tape                                           
17:37:06 verbose:                     | > eclint check $(git ls-files 2>/dev/null | xargs find 2> /dev/null | grep -vE 'node_modules|\.git' || echo '*.md *.js test/*.js')
17:37:06 verbose: tape npm-test:      | events.js:377                                                                                                                                                          
17:37:06 verbose:                     | throw er; // Unhandled 'error' event                                                                                                                                   
17:37:06 verbose:                     | ^                                                                                                                                                                      
17:37:06 verbose:                     |                                                                                                                                                                        
17:37:06 verbose:                     | Error: File not found with singular glob: /home/iojs/tmp/citgm_tmp/bef89702-9128-4153-9811-33d5cef3002d/tape/test/has (if this was purposeful, use `allowEmpty` option)

https://github.com/substack/tape/archive/HEAD.tar.gz doesn't contain a .git directory. I think we're in the second half of $(git ls-files 2>/dev/null | xargs find 2> /dev/null | grep -vE 'node_modules|\.git' || echo '*.md *.js test/*.js') -- i.e. $(echo '*.md *.js test/*.js').

FWIW we could make CITGM use git to clone tape rather than use the tarball using the useGitClone option in the lookup file.

diff --git a/lib/lookup.json b/lib/lookup.json
index 1f43084..d19636d 100644
--- a/lib/lookup.json
+++ b/lib/lookup.json
@@ -506,7 +506,8 @@
   "tape": {
     "head": true,
     "prefix": "v",
-    "maintainers": "substack"
+    "maintainers": "substack",
+    "useGitClone": "true"
   },
   "thread-sleep": {
     "install": ["install", "--build-from-source"],
ljharb commented 2 years ago

ahhhhh well yeah, if you're using the tarball then it's not necessarily going to include all the files needed to run the tests.

Seems like the simplest option is to enable useGitClone :-D

ljharb commented 2 years ago

Although, I'll first change tape so that an empty file list doesn't break eclint.

ljharb commented 2 years ago

alrighty - try https://github.com/substack/tape/commit/5f781346aa7cd7eb6a14b532304787cbc7287b9c and hopefully that will gracefully avoid the eclint failure when there's no .git directory.

BethGriggs commented 2 years ago

That seems to have worked 🎉 - https://ci.nodejs.org/job/citgm-smoker-nobuild/nodes=ubuntu1804-64/1163/console

ljharb commented 2 years ago

yay! let's just stick with that then. Thanks for the error output, and thanks to @richardlau for figuring out the source of the problem :-)

Is this OK to close?

BethGriggs commented 2 years ago

Actually, windows is still unhappy: 

22:52:34 error:                     | > tape@5.5.2 prepublish                                                          
22:52:34 error:                     | > !(type not-in-publish) || not-in-publish || npm run prepublishOnly             
22:52:34 error:                     |                                                                                  
22:52:34 error:                     |                                                                                  
22:52:34 error:                     | added 761 packages in 1m                                                         
22:52:34 error:                     |                                                                                  
22:52:34 error:                     | > tape@5.5.2 pretest                                                             
22:52:34 error:                     | > npm run lint                                                                   
22:52:34 error:                     |                                                                                  
22:52:34 error:                     |                                                                                  
22:52:34 error:                     | > tape@5.5.2 prelint                                                             
22:52:34 error:                     | > FILES="$(npm run --silent prelint:files)" eclint check "${FILES:=package.json}"
22:52:34 error:                     |                                                                                  
22:52:34 error:                     |                                                                                  
22:52:34 error:                     | '!' is not recognized as an internal or external command,                        
22:52:34 error:                     | operable program or batch file.                                                  
22:52:34 error:                     | 'FILES' is not recognized as an internal or external command,                    
22:52:34 error:                     | operable program or batch file.
ljharb commented 2 years ago

ha, fun, env vars and also subcommands are linux-only commands. i'll see if i can figure out a clean way to make that script a noop on windows.

ljharb commented 2 years ago

@BethGriggs at your convenience, please run another windows build - i think the latest commit should solve it.

BethGriggs commented 2 years ago

It seems to have gotten much further, but still bailing out of the job at the end on Windows 🤔 - https://ci.nodejs.org/job/citgm-smoker-nobuild/nodes=win10-vs2019/1171/consoleText

ljharb commented 2 years ago

Error: Cannot find module 'C:\Program Files\Git\citgm_tmp\deda5417-7299-40c9-a86b-025bc9b697c8\noname-e804b1b8e297c76799e7371f36dae70d794bdc30\node'

im not sure what that means; node is clearly there since eslint just ran prior, but the tests aren’t finding it?

ljharb commented 2 years ago

I got the access I needed in https://github.com/nodejs/build/issues/2913, and I think I fixed this with https://github.com/substack/tape/commit/3e7b2ae9800964cf8461ab8dc10634d0c1b1218a - any chance someone can let me know if the next CIGTM run still has this failure?

update: nvm, this might not be fixed yet; still working on it.