Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tape failing on multiple versions and platforms #895

Open
Tracked by #894
BethGriggs opened this issue Apr 5, 2022 · 19 comments
Open
Tracked by #894

tape failing on multiple versions and platforms #895

BethGriggs opened this issue Apr 5, 2022 · 19 comments

Comments

@BethGriggs
Copy link
Member

BethGriggs commented Apr 5, 2022

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

Most platforms fail with this error [Resolved]:

> [email protected] 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
> [email protected] pretest
> npm run lint
> [email protected] 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:

 > [email protected] prepublish
> !(type not-in-publish) || not-in-publish || npm run prepublishOnly
added 759 packages in 34s
> [email protected] pretest
> npm run lint
> [email protected] 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
Copy link
Member Author

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

@ljharb
Copy link
Member

ljharb commented Apr 5, 2022

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
Copy link
Member

ljharb commented Apr 5, 2022

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
Copy link
Member Author

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 nodejs/build#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
Copy link
Member

ljharb commented Apr 5, 2022

Should i file an issue for one of these too?

@BethGriggs
Copy link
Member Author

Sure, or maybe you could rename nodejs/build#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 BethGriggs mentioned this issue Apr 5, 2022
21 tasks
@BethGriggs
Copy link
Member Author

@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
Copy link
Member

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:   | > [email protected] 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:      | > [email protected] 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:      | > [email protected] 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
Copy link
Member

ljharb commented Apr 5, 2022

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
Copy link
Member

ljharb commented Apr 5, 2022

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

@ljharb
Copy link
Member

ljharb commented Apr 5, 2022

alrighty - try tape-testing/tape@5f78134 and hopefully that will gracefully avoid the eclint failure when there's no .git directory.

@BethGriggs
Copy link
Member Author

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

@ljharb
Copy link
Member

ljharb commented Apr 5, 2022

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
Copy link
Member Author

Actually, windows is still unhappy:

22:52:34 error:                     | > [email protected] 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:                     | > [email protected] pretest                                                             
22:52:34 error:                     | > npm run lint                                                                   
22:52:34 error:                     |                                                                                  
22:52:34 error:                     |                                                                                  
22:52:34 error:                     | > [email protected] 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.  

@BethGriggs BethGriggs reopened this Apr 5, 2022
@ljharb
Copy link
Member

ljharb commented Apr 5, 2022

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
Copy link
Member

ljharb commented Apr 6, 2022

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

@BethGriggs
Copy link
Member Author

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
Copy link
Member

ljharb commented Apr 6, 2022

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
Copy link
Member

ljharb commented Apr 25, 2022

I got the access I needed in nodejs/build#2913, and I think I fixed this with tape-testing/tape@3e7b2ae - 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants