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

lib: log correct version and sha for -c option #1026

Merged
merged 1 commit into from
Nov 14, 2023

Conversation

BethGriggs
Copy link
Member

The main motivation here is to fix misleading version output when using the -c option.

$ npx citgm -v verbose -c 37c34bad omg-i-pass 
info: starting            | omg-i-pass          
verbose: omg-i-pass using-uid| 501                 
verbose: omg-i-pass using-gid| 20                  
verbose: omg-i-pass using-node| /Users/bgriggs/.nvm/versions/node/v20.9.0/bin/node
verbose: omg-i-pass mk.tempdir| /var/folders/9g/93qrln9j5yq3396zcjzsghf00000gn/T/11c4180e-ff8e-4b33-8641-5dfa4dc76c80
verbose: omg-i-pass npm-view | Retrieving Meta Data
verbose: omg-i-pass npm-view | Data retrieved      
info: lookup              | omg-i-pass          
info: lookup-notfound     | omg-i-pass          
info: lookup-githead      | https://github.com/TheAlphaNerd/omg-i-pass/archive/37c34bad.tar.gz
info: omg-i-pass npm:     | Downloading project: https://github.com/TheAlphaNerd/omg-i-pass/archive/37c34bad.tar.gz
info: omg-i-pass npm:     | Project downloaded 37c34bad.tar.gz
info: omg-i-pass npm:     | npm install started 
verbose: omg-i-pass npm:     | Using temp directory: "/var/folders/9g/93qrln9j5yq3396zcjzsghf00000gn/T/11c4180e-ff8e-4b33-8641-5dfa4dc76c80/npm_config_
verbose: omg-i-pass npm-install:| up to date in 103ms 
info: omg-i-pass npm:     | npm install successfully completed
info: omg-i-pass npm:     | test suite started  
verbose: omg-i-pass npm-test:| > [email protected] test
verbose:                     | > exit 0               
info: passing module(s)   |                     
info: module name:        | omg-i-pass          
info: version:            | 3.0.0               
info: done                | The smoke test has passed.
info: duration            | test duration: 1666ms

Note that [email protected] test is what was run as expected (corresponding to MylesBorins/omg-i-pass@37c34ba) but CITGM reports it as latest: info: version: | 3.0.0.

With this change:

$ node ./bin/citgm -v verbose -c 37c34bad omg-i-pass 
info: starting            | omg-i-pass          
verbose: omg-i-pass using-uid| 501                 
verbose: omg-i-pass using-gid| 20                  
verbose: omg-i-pass using-node| /Users/bgriggs/.nvm/versions/node/v20.9.0/bin/node
verbose: omg-i-pass mk.tempdir| /var/folders/9g/93qrln9j5yq3396zcjzsghf00000gn/T/954b0474-9012-486c-b7d8-a4cec87a279e
verbose: omg-i-pass npm-view | Retrieving Meta Data
verbose: omg-i-pass npm-view | Data retrieved      
info: lookup              | omg-i-pass          
info: lookup-notfound     | omg-i-pass          
info: lookup-githead      | https://github.com/TheAlphaNerd/omg-i-pass/archive/37c34bad.tar.gz
info: omg-i-pass npm:     | Downloading project: https://github.com/TheAlphaNerd/omg-i-pass/archive/37c34bad.tar.gz
info: omg-i-pass npm:     | Project downloaded 37c34bad.tar.gz
info: omg-i-pass npm:     | npm install started 
verbose: omg-i-pass npm:     | Using temp directory: "/var/folders/9g/93qrln9j5yq3396zcjzsghf00000gn/T/954b0474-9012-486c-b7d8-a4cec87a279e/npm_config_
verbose: omg-i-pass npm-install:| up to date in 128ms 
info: omg-i-pass npm:     | npm install successfully completed
info: omg-i-pass npm:     | test suite started  
verbose: omg-i-pass npm-test:| > [email protected] test
verbose:                     | > exit 0               
info: passing module(s)   |                     
info: module name:        | omg-i-pass          
info: version:            | 2.0.1               
info: sha:                | 37c34bad            
info: done                | The smoke test has passed.
info: duration            | test duration: 1725ms

I also added info: sha: | 37c34bad as extra log information only when using the -c option because it's possible multiple commits will reference the same version, and the explicit debug information seems like it would be useful.

This was a bit of a 🐰 🕳️ so if anyone has a better suggestion for how to correct this feel free 🙂

Checklist
  • npm test passes
  • tests are included
  • contribution guidelines followed
    here

When using the -c option to supply a specific sha to test CITGM will
erroneously log the latest version of the module. This commit overrides
the incorrect value with the one that is in the downloaded package.json.
Additionally, when supplying -c CITGM will now output the sha for easier
debugging.
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (eae7346) 96.44% compared to head (2c96cdc) 96.45%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1026   +/-   ##
=======================================
  Coverage   96.44%   96.45%           
=======================================
  Files          28       28           
  Lines        2139     2143    +4     
=======================================
+ Hits         2063     2067    +4     
  Misses         76       76           
Files Coverage Δ
lib/citgm.js 100.00% <100.00%> (ø)
lib/lookup.js 91.78% <100.00%> (-0.12%) ⬇️
lib/package-manager/test.js 99.36% <100.00%> (+<0.01%) ⬆️
lib/reporter/logger.js 77.21% <100.00%> (+0.59%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@BethGriggs BethGriggs merged commit 18cf7df into nodejs:main Nov 14, 2023
8 checks passed
@BethGriggs BethGriggs deleted the shabug branch November 15, 2023 02:22
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

Successfully merging this pull request may close these issues.

4 participants