-
Notifications
You must be signed in to change notification settings - Fork 252
Enable coverage reporting #3100
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
Changes from 9 commits
b82d545
89ac4c7
d58f234
d3b2549
5877439
93279c1
6d1b9c7
3192156
856abbe
464b464
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,7 +14,7 @@ jobs: | |
runs-on: ubuntu-latest | ||
strategy: | ||
matrix: | ||
node-version: ['12.14.1', '12.x', '14.x'] | ||
node-version: ['12.x', '14.x'] | ||
steps: | ||
- uses: actions/checkout@v2 | ||
with: | ||
|
@@ -117,6 +117,58 @@ jobs: | |
name: benchmarkstats.json | ||
path: packages/swingset-runner/benchstats*.json | ||
|
||
coverage: | ||
needs: build | ||
runs-on: ubuntu-latest | ||
strategy: | ||
matrix: | ||
node-version: ['14.x'] | ||
# TODO: if: ${{github.event_name == 'push' && github.ref == 'refs/heads/master'}} | ||
steps: | ||
- uses: actions/setup-node@v1 | ||
with: | ||
node-version: ${{ matrix.node-version }} | ||
# BEGIN-RESTORE-BOILERPLATE | ||
- name: restore built files | ||
id: built | ||
uses: actions/cache@v1 | ||
with: | ||
path: . | ||
key: ${{ runner.os }}-${{ matrix.node-version }}-built-${{ github.sha }} | ||
- uses: actions/checkout@v2 | ||
with: | ||
submodules: 'true' | ||
if: steps.built.outputs.cache-hit != 'true' | ||
- name: yarn install | ||
run: yarn install | ||
if: steps.built.outputs.cache-hit != 'true' | ||
- name: yarn build | ||
run: yarn build | ||
if: steps.built.outputs.cache-hit != 'true' | ||
# END-RESTORE-BOILERPLATE | ||
|
||
- name: generate coverage for all tests | ||
run: 'yarn test:c8-all || :' | ||
- name: generate coverage/html reports | ||
run: yarn c8 report --reporter=html-spa --reports-dir=coverage/html --temp-directory=coverage/tmp | ||
- uses: actions/upload-artifact@v2 | ||
with: | ||
name: coverage | ||
path: coverage | ||
- name: Find Netlify site ID | ||
run: | | ||
echo "NETLIFY_SITE_ID=$(cat COVERAGE_NETLIFY_SITE_ID)" >> $GITHUB_ENV | ||
|
||
- uses: nwtgck/[email protected] | ||
with: | ||
# Production deployment if a commit to master. | ||
production-deploy: ${{ github.ref != 'TODOrefs/heads/master' }} | ||
publish-dir: coverage/html | ||
# SECURITY: we don't want to hand out the Github token to this action. | ||
# github-token: ${{ secrets.GITHUB_TOKEN }} | ||
env: | ||
NETLIFY_AUTH_TOKEN: ${{ secrets.NETLIFY_AUTH_TOKEN }} | ||
|
||
################## | ||
# Fast-running tests run as a group: | ||
test-quick: | ||
|
@@ -125,7 +177,7 @@ jobs: | |
runs-on: ubuntu-latest | ||
strategy: | ||
matrix: | ||
node-version: ['12.14.1', '12.x', '14.x'] | ||
node-version: ['12.x', '14.x'] | ||
steps: | ||
- uses: actions/setup-node@v1 | ||
with: | ||
|
@@ -254,7 +306,7 @@ jobs: | |
runs-on: ubuntu-latest | ||
strategy: | ||
matrix: | ||
node-version: ['12.14.1', '12.x', '14.x'] | ||
node-version: ['12.x', '14.x'] | ||
steps: | ||
- uses: actions/setup-node@v1 | ||
with: | ||
|
@@ -320,7 +372,7 @@ jobs: | |
runs-on: ubuntu-latest | ||
strategy: | ||
matrix: | ||
node-version: ['12.14.1', '12.x', '14.x'] | ||
node-version: ['12.x', '14.x'] | ||
steps: | ||
- uses: actions/setup-node@v1 | ||
with: | ||
|
@@ -358,7 +410,7 @@ jobs: | |
runs-on: ubuntu-latest | ||
strategy: | ||
matrix: | ||
node-version: ['12.14.1', '12.x', '14.x'] | ||
node-version: ['12.x', '14.x'] | ||
steps: | ||
- uses: actions/setup-node@v1 | ||
with: | ||
|
@@ -405,7 +457,7 @@ jobs: | |
runs-on: ubuntu-latest | ||
strategy: | ||
matrix: | ||
node-version: ['12.14.1', '12.x', '14.x'] | ||
node-version: ['12.x', '14.x'] | ||
steps: | ||
- uses: actions/setup-node@v1 | ||
with: | ||
|
@@ -450,7 +502,7 @@ jobs: | |
runs-on: ubuntu-latest | ||
strategy: | ||
matrix: | ||
node-version: ['12.14.1', '12.x', '14.x'] | ||
node-version: ['12.x', '14.x'] | ||
steps: | ||
- uses: actions/setup-node@v1 | ||
with: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,72 @@ | ||
# Coverage reports | ||
|
||
## Caveat | ||
|
||
Until each module can be migrated to support Node.js's builtin ESM | ||
implementation (`nesm`), the coverage line numbers will be out-of-sync with | ||
reality. | ||
|
||
In addition, we will have to implement source maps in all of our | ||
source-to-source transforms (such as `@agoric/bundle-source`, | ||
`@agoric/transform-metering`, and `@agoric/static-module-record`). | ||
|
||
## Reports | ||
|
||
Coverage reports for the current main branch (whose packages support `nesm`) are | ||
published by CI to: https://agoric-sdk-coverage.netlify.app | ||
|
||
You can create a report in any package (including the top-level directory): | ||
|
||
```sh | ||
# Get options available for coverage: | ||
yarn c8 --help | ||
# Run ava under Node.js coverage and display a summary: | ||
yarn c8 -a ava | ||
# Generate a nice, detailed HTML report: | ||
yarn c8 report --reporter=html-spa | ||
open coverage/html/index.html | ||
``` | ||
|
||
## Node.js ESM Support | ||
|
||
With the current `patches/esm+3.2.25.diff`, it is possible to migrate packages | ||
to support both resm (`-r esm`) and nesm (Node.js ESM Support). If an | ||
`agoric-sdk` package has dependencies that support nesm, you can attempt to make | ||
it also support nesm by: | ||
|
||
1. Create `ava-nesm.config.js` removing `"require": ["esm"]`: | ||
|
||
```sh | ||
../../scripts/ava-nesm.cjs > ava-nesm.config.js | ||
``` | ||
|
||
2. Make the following changes to its `package.json` (omitting comments): | ||
|
||
```json | ||
{ | ||
// Enable nesm support. | ||
"type": "module", | ||
"scripts": { | ||
// The following line enables coverage generation from the top. | ||
"test:c8": "c8 $C8_OPTIONS ava --config=ava-nesm.config.js", | ||
michaelfig marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
} | ||
} | ||
``` | ||
|
||
3. Test that both `yarn test` and `yarn test:c8` run correctly. | ||
|
||
## Planned Implementation | ||
|
||
Our runtime source transforms can be conditional on the `$NODE_V8_COVERAGE` | ||
environment variable, which is set by `c8`. | ||
|
||
When that is nonempty, source transforms can implement special behaviour to | ||
preserve source maps for transformed code. This involves using a `//# | ||
sourceURL=...` tag to associate a unique URL to an actual existing file during | ||
`eval`. The file's contents include a `//# sourceMappingURL=...`, whether a | ||
relative URL (taken from `sourceURL`) to a `.js.map` file or inline as a `data:` | ||
URI. | ||
|
||
|
||
The `sourceMappingURL` source map must contain a `"sources"` property as a | ||
relative or absolute URL to the input source file. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
91d78371-7395-4779-8367-0c4f7088297c |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -49,6 +49,7 @@ | |
"devDependencies": { | ||
"@typescript-eslint/parser": "^4.18.0", | ||
"ava": "^3.12.1", | ||
"c8": "^7.7.2", | ||
"eslint": "^7.23.0", | ||
"eslint-config-airbnb-base": "^14.0.0", | ||
"eslint-config-jessie": "^0.0.4", | ||
|
@@ -58,7 +59,6 @@ | |
"eslint-plugin-jsx-a11y": "^6.2.3", | ||
"eslint-plugin-prettier": "^3.1.2", | ||
"lerna": "^3.20.2", | ||
"nyc": "^15.1.0", | ||
"prettier": "^1.18.2", | ||
"typescript": "^4.2.3" | ||
}, | ||
|
@@ -75,13 +75,23 @@ | |
"lint-fix": "yarn workspaces run lint-fix", | ||
"lint-check": "yarn workspaces run lint-check", | ||
"lint": "yarn workspaces run lint-check", | ||
"test": "yarn workspaces run test", | ||
"test": "ava", | ||
"test:c8-all": "rm -rf coverage/tmp && C8_OPTIONS=\"--clean=false --temp-directory=$PWD/coverage/tmp\" lerna run test:c8", | ||
"test:xs": "yarn workspaces run test:xs", | ||
"build": "yarn workspaces run build", | ||
"postinstall": "patch-package", | ||
"patch-package": "patch-package", | ||
"build-xs-worker": "cd packages/xs-vat-worker && yarn build:xs-lin" | ||
}, | ||
"ava": { | ||
"files": [ | ||
"packages/*/test/**/test-*.js" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this interfere with / override It looks like this is meant for use by a top-level There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Only when run from the top level. If
Yes. It turns out we only use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, I think @dtribble was once keen on using the AVA config to jam a I've got |
||
], | ||
"require": [ | ||
"esm" | ||
], | ||
"timeout": "30m" | ||
}, | ||
"dependencies": { | ||
"patch-package": "^6.2.2" | ||
} | ||
|
This file was deleted.
Uh oh!
There was an error while loading. Please reload this page.