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

Add JS code coverage option to front end tests #3036

Merged
merged 2 commits into from May 26, 2020
Merged

Add JS code coverage option to front end tests #3036

merged 2 commits into from May 26, 2020

Conversation

dracos
Copy link
Member

@dracos dracos commented May 18, 2020

  • All cobrand-specific commits start their commit message with the cobrand in square brackets;
  • Is new functionality tested? CodeCov will warn you about the diff coverage, but won’t complain about e.g. new files;
  • Have you updated the changelog? If this is not necessary, put square brackets around this: skip changelog

This PR adds a way to view code coverage of the JS we use locally if you'd like to do that. Works for me with WSL on Windows, asked for two reviews as I know e.g. Zarino uses Vagrant. Produces output like the screenshot.

@codecov
Copy link

codecov bot commented May 18, 2020

Codecov Report

Merging #3036 into master will increase coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #3036   +/-   ##
=======================================
  Coverage   83.07%   83.07%           
=======================================
  Files         246      246           
  Lines       15429    15429           
  Branches     2886     2886           
=======================================
+ Hits        12817    12818    +1     
  Misses       1695     1695           
+ Partials      917      916    -1     
Impacted Files Coverage Δ
perllib/Utils.pm 98.98% <0.00%> (+1.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 62c9392...36bdf3a. Read the comment docs.

Copy link
Member

@struan struan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incorrect executable name on Mac, although that might be me installing things incorrectly.

Also worth noting that I had to upgrade my, somewhat old, cypress for this to work. I was on 3.2.


# Get a path for nyc
my $nyc = 'nyc';
$nyc = abs_path("$wsl/../../../\@cypress/code-coverage/node_modules/nyc/bin/nyc.js") if $wsl;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this gets installed as nyc.js on my mac.

I also had to add the location to the path which was /Users/$username/node_modules/nyc/bin

Copy link
Member Author

@dracos dracos May 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will be "nyc" in your /Users/$username/node_modules/.bin I think?

Did you install "nyc" yourself or only the code-coverage package?

On mine it is in node_modules/@cypress/code-coverage/node_modules/.bin/nyc and corresponding node_modules/@cypress/code-coverage/node_modules/nyc/bin/nyc.js

So if I know where cypress is installed I could do the relative path from that which should be ../../../@cypress/code-coverage/node_modules/.bin/nyc, but do I know that with any certainty? Hmm.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, of course you have a hidden bin directory. That totally makes sense.

Anyway, it is indeed in /Users/$username/node_modules/.bin so adding that to my path works fine. It's not in node_modules/@cypress/code-coverage/node_modules/.bin/ though.

I've uninistalled nyc and the code coverage module and then re-installed the code-coverage module to get this as the end result FWIW.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, just for the record, here's the final skinny as I understand it.

Facts:

  • All I want is a path to run nyc.
  • The code already assumes (for non-WSL) cypress is on PATH, and you specify a path to cypress with WSL.

Where is nyc installed:

  • If you install cypress and code-coverage with npm locally (no -g flag), with nothing else installed, then npm will install them in any node_modules directory it finds, up to your homedir. In that node_modules directory, cypress and nyc can be run from node_modules/.bin.
  • However, if e.g. you already had an older version of nyc manually installed, then that version would be the one in node_modules/.bin and the code-coverage packaged version would be in @cypress/code-coverage/node_modules/.bin!
  • If you install cypress and code-coverage with npm globally (with -g flag), then npm will install them in your "prefix" (probably /usr/local/lib/node_modules or AppData\npm\node_modules on Windows, or ~/.npm/node_modules for me because I have set that somewhere). It creates top-level executables in /usr/local/bin on Unix, or AppData\npm\ directly on Windows - so "nyc" doesn't get a executable as it's a dependency, all I have there is cypress. nyc can be found only as nyc.js in @cypress/code-coverage/node_modules/nyc/bin

Possible locations therefore of nyc or nyc.js:

  • $(npm prefix)/node_modules/@cypress/code-coverage/node_modules/.bin/nyc
  • $(npm prefix)/node_modules/.bin/nyc
  • $(npm prefix -g)/node_modules/@cypress/code-coverage/node_modules/.bin/nyc
  • $(npm prefix -g)/[bin/]nyc (if you manually install nyc globally)

Possible locations of cypress, which you must have added to your path to get this to run previously anyway (or be using WSL like me):

  • $(npm prefix)/node_modules/.bin/cypress
  • $(npm prefix -g)/[bin/]cypress

I think easiest would be to ask the user to install nyc manually (globally or not), and then assume the right dir is on their PATH and then this script doesn't have to care.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. I think if you are running a code coverage tool you should be tech savy enough to include node modules in your path.

@zarino
Copy link
Member

zarino commented May 18, 2020

Leaving a note here that I can‘t seem to get this working on my Mac.

$ bin/browser-tests --coverage --vagrant run
Waiting for test server.............Created body Borsetshire for MapIt area ID 2608, categories Abandoned vehicles, Bus stops, Dog fouling, Flyposting, Flytipping, Footpath/bridleway away from road, Graffiti, Parks/landscapes, Pavements, Potholes, Public toilets, Roads/highways, Road traffic signs, Rubbish (refuse and recycling), Street cleaning, Street lighting, Street nameplates, Traffic lights, Trees, Other
[…]
Created updates on problems 14, 14, 14, 14, 14, 14, 14, 14, 14, 14, 14, 14, 14, 9, 9, 9, 9, 9, 9, 9, 9, 3, 3, 4, 10, 21
.... done
The function exported by the plugins file threw an error.

We invoked the function exported by `/Users/zarinozappia/Work/mySociety/fixmystreet/fixmystreet/.cypress/cypress/plugins/index.js`, but it threw an error.

 Error: Cannot find module '@cypress/code-coverage/task'
Require stack:
- /Users/zarinozappia/Work/mySociety/fixmystreet/fixmystreet/.cypress/cypress/plugins/index.js
- /Users/zarinozappia/Library/Caches/Cypress/4.5.0/Cypress.app/Contents/Resources/app/packages/server/lib/plugins/child/run_plugins.js
- /Users/zarinozappia/Library/Caches/Cypress/4.5.0/Cypress.app/Contents/Resources/app/packages/server/lib/plugins/child/index.js
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:798:15)
    at Module._load (internal/modules/cjs/loader.js:691:27)
    at Module._load (electron/js2c/asar.js:717:26)
    at Function.Module._load (electron/js2c/asar.js:717:26)
    at Module.require (internal/modules/cjs/loader.js:853:19)
    at require (internal/modules/cjs/helpers.js:74:18)
    at module.exports (/Users/zarinozappia/Work/mySociety/fixmystreet/fixmystreet/.cypress/cypress/plugins/index.js:4:7)
    at /Users/zarinozappia/Library/Caches/Cypress/4.5.0/Cypress.app/Contents/Resources/app/packages/server/lib/plugins/child/run_plugins.js:77:12
    at tryCatcher (/Users/zarinozappia/Library/Caches/Cypress/4.5.0/Cypress.app/Contents/Resources/app/packages/server/node_modules/bluebird/js/release/util.js:16:23)
    at Function.Promise.attempt.Promise.try (/Users/zarinozappia/Library/Caches/Cypress/4.5.0/Cypress.app/Contents/Resources/app/packages/server/node_modules/bluebird/js/release/method.js:39:29)
    at load (/Users/zarinozappia/Library/Caches/Cypress/4.5.0/Cypress.app/Contents/Resources/app/packages/server/lib/plugins/child/run_plugins.js:74:7)
    at EventEmitter.<anonymous> (/Users/zarinozappia/Library/Caches/Cypress/4.5.0/Cypress.app/Contents/Resources/app/packages/server/lib/plugins/child/run_plugins.js:229:5)
    at EventEmitter.emit (events.js:210:5)
    at process.<anonymous> (/Users/zarinozappia/Library/Caches/Cypress/4.5.0/Cypress.app/Contents/Resources/app/packages/server/lib/plugins/util.js:25:29)
    at process.emit (events.js:210:5)
    at process.emit (/Users/zarinozappia/Library/Caches/Cypress/4.5.0/Cypress.app/Contents/Resources/app/packages/server/node_modules/source-map-support/source-map-support.js:495:21)
Updated 69 paths from the index
ENOENT: no such file or directory, scandir '/Users/zarinozappia/Work/mySociety/fixmystreet/fixmystreet/.nyc_output'
The JS coverage report is at coverage/lcov-report/index.html

That’s with --coverage hard-coded into the vagrant inception bit of bin/browser-tests:

$ git diff bin/browser-tests
diff --git a/bin/browser-tests b/bin/browser-tests
index 1289f4d10..985e8df0f 100755
--- a/bin/browser-tests
+++ b/bin/browser-tests
@@ -76,7 +76,7 @@ if ($vagrant) {
         print '.'; sleep 1;
     }
     print " done\n";
-    system('bin/browser-tests', '--cypress', @ARGV);
+    system('bin/browser-tests', '--coverage', '--cypress', @ARGV);
     system('vagrant', 'ssh', '--', 'kill $(cat /tmp/cypress-server.pid)');
     exit;
 }

And the following versions of everything:

$ node -v
v14.2.0
$ npm -v
6.14.4
$ npm list -g --depth=0
/usr/local/lib
├── @cypress/code-coverage@3.7.4
├── cypress@4.5.0
├── localtunnel@1.9.0
├── npm@6.14.4
└── nyc@15.0.1

@dracos notes that our tests might not work with Cypress 4 yet, so I should probably downgrade to 3.8.3 – but it seems unlikely this is the cause of the "cannot find module" error.

You need to install the @cypress/code-coverage package alongside cypress.
@dracos dracos merged commit 17ed2f8 into master May 26, 2020
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.

None yet

3 participants