Skip to content

Conversation

@ChristopherDedominici
Copy link
Contributor

@ChristopherDedominici ChristopherDedominici commented Oct 8, 2025

Link to internal design doc: https://www.notion.so/nomicfoundation/Code-coverage-HTML-report-286578cdeaf580bcad22db58ce54fb47?source=copy_link

As listed in the design doc, this PR addresses point number 1 out of 3:

  1. create the HTML report
  2. investigate and fix the coverage bug
  3. refactor the code to separate coverage data collection and report generation

TODOs before merge

  • add mention to vendored code

NOTEs for review

  • All the code inside the folder v-next/hardhat-vendored/src/coverage-module has been copied from vendored packages, so there’s no need for a deep review, a quick scan will be sufficient

@changeset-bot
Copy link

changeset-bot bot commented Oct 8, 2025

🦋 Changeset detected

Latest commit: b26e193

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions
Copy link
Contributor

github-actions bot commented Oct 8, 2025

hardhat

Total size of the bundle: 236M
Total number of dependencies (including transitive): 48

List of dependencies (sorted by size)
229M	total
35M	@nomicfoundation/edr-linux-x64-musl
35M	@nomicfoundation/edr-linux-x64-gnu
32M	@nomicfoundation/edr-linux-arm64-musl
32M	@nomicfoundation/edr-linux-arm64-gnu
24M	@nomicfoundation/edr-win32-x64-msvc
24M	@nomicfoundation/edr-darwin-x64
20M	@nomicfoundation/edr-darwin-arm64
7.3M	@sentry/core
5.2M	zod
2.7M	micro-eth-signer
1.9M	@noble/curves
1.7M	undici
1.2M	@noble/hashes
1020K	@nomicfoundation/hardhat-utils
868K	@nomicfoundation/hardhat-vendored
864K	@streamparser/json
624K	micro-packed
592K	tsx
548K	@nomicfoundation/hardhat-errors
492K	@scure/bip39
464K	@nomicfoundation/edr
452K	fast-equals
408K	json-stream-stringify
368K	ethereum-cryptography
332K	@streamparser/json-node
320K	enquirer
320K	@nomicfoundation/hardhat-zod-utils
288K	semver
200K	ws
180K	chokidar
176K	get-tsconfig
168K	@scure/base
160K	esbuild
136K	adm-zip
96K	@scure/bip32
92K	chalk
72K	@nomicfoundation/solidity-analyzer
68K	debug
60K	readdirp
56K	rfdc
48K	ansi-colors
44K	resolve.exports
40K	resolve-pkg-maps
36K	p-map
24K	strip-ansi
24K	env-paths
24K	ansi-regex
20K	ms

@@ -0,0 +1,2 @@
import libCoverage from "./lib-coverage/index.cjs";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a "bridge" to expose the common js code into ESM js code, so that the ts file can then import it

"pretest": "pnpm build",
"pretest:only": "pnpm build",
"build": "tsc --build .",
"postbuild": "node dist/src/copy-assets.js",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This step is required to copy the assets folder, which contains the CSS and JS files used to generate the report, into the dist folder after compilation

@@ -0,0 +1,33 @@
import { cp } from "node:fs/promises";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This function is invoked by the postbuild in the package.json file.

Used to copy the assets folder, which contains the CSS and JS files used to generate the report, into the dist folder after compilation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm open to better ideas to solve this issue

@@ -0,0 +1,13 @@
{
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I need a double-check here. I copied the configuration from tsconfig.base.json, but removed all the properties that would cause errors during compilation, since this folder contains non-ESM files

report: Report,
htmlReportPath: string,
): Promise<void> {
const baseDir = process.cwd();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since there is a bug in how we generate coverage data, we currently only display executed and non-executed lines. The line counter is either 1 for executed lines or 0 for non-executed lines:

Image

This will be improved after fixing how we collect coverage data

@ChristopherDedominici ChristopherDedominici marked this pull request as ready for review October 28, 2025 13:36
@ChristopherDedominici ChristopherDedominici moved this from In Progress to In Review in Hardhat Oct 28, 2025
@ChristopherDedominici ChristopherDedominici added the no docs needed This PR doesn't require links to documentation label Nov 3, 2025
@ChristopherDedominici ChristopherDedominici linked an issue Nov 3, 2025 that may be closed by this pull request
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This file is intentionally empty to prevent build errors and avoid changing the default scripts in package.json

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no docs needed This PR doesn't require links to documentation

Projects

Status: In Review

Development

Successfully merging this pull request may close these issues.

Generate HTML coverage report

3 participants