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.

Sign up for GitHub

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

Jump to bottom

add skeleton EDR Solidity Testing integration #5780

Merged
merged 6 commits into from
Oct 1, 2024
Merged

add skeleton EDR Solidity Testing integration #5780

merged 6 commits into from
Oct 1, 2024

Conversation

kanej
Copy link
Member

@kanej kanej commented Sep 26, 2024 •
edited
Loading

Add a skeleton Solidity Test built-in plugin using @ignored/edr with its Solidity Test Runner, to add a npx hardhat test:solidity task.

This PR comes in two commits:

  • temporary hack to add compile task based on old build system and extend ArtifactsManager to read from artifacts directory
  • skeleton builtin plugin for Solidity Test, leveraging the hacked artifacts manager to read the compiled artifacts and pass that meta-info into EDR's Solidity Test Runner

Running locally

pnpm install
cd ./v-next/example-project
pnpm build
pnpm hardhat test:solidity

@changeset-bot changeset-bot
Copy link

changeset-bot bot commented Sep 26, 2024 •
edited
Loading

⚠️ No Changeset found

Latest commit: 16d7c13

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

@vercel Vercel
Copy link

vercel bot commented Sep 26, 2024 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
hardhat ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 1, 2024 9:37am

@github-actions github-actions bot added the status:ready This issue is ready to be worked on label Sep 26, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Sep 26, 2024 •
edited
Loading

hardhat

Total size of the bundle: 80M
Total number of dependencies (including transitive): 120

List of dependencies (sorted by size) 
77M	total
23M	solc
20M	esbuild
4.9M	lodash
2.9M	fp-ts
2.8M	@sentry/tracing
2.1M	secp256k1
1.9M	@noble/curves
1.6M	undici
1.2M	@sentry/types
1.2M	@noble/hashes
1.1M	@ignored/hardhat-vnext-build-system
996K	@nomicfoundation/ethereumjs-util
952K	node-addon-api
932K	@sentry/node
920K	@sentry/utils
892K	keccak
824K	zod
712K	@ignored/hardhat-vnext-utils
576K	tsx
556K	resolve
548K	@sentry/core
504K	fast-equals
492K	@scure/bip39
464K	@ignored/edr
368K	ethereum-cryptography
344K	@sentry/hub
320K	enquirer
284K	semver
268K	fs-extra
248K	scrypt-js
244K	io-ts
200K	undici-types
196K	readable-stream
192K	@nomicfoundation/ethereumjs-rlp
192K	@ignored/hardhat-vnext-errors
180K	blakejs
176K	elliptic
168K	@scure/base
136K	adm-zip
128K	get-tsconfig
112K	bn.js
104K	hash.js
96K	commander
96K	browserify-aes
96K	@scure/bip32
92K	chalk
88K	tslib
88K	@sentry/minimal
84K	js-sha3
76K	agent-base
72K	sha.js
72K	@nomicfoundation/solidity-analyzer
68K	glob
68K	debug
64K	lru_map
64K	https-proxy-agent
60K	@ignored/hardhat-vnext-zod-utils
56K	rfdc
56K	graceful-fs
56K	follow-redirects
52K	pbkdf2
52K	hmac-drbg
48K	safe-buffer
48K	minimatch
48K	memorystream
48K	command-exists
48K	ansi-colors
44K	tmp
44K	node-gyp-build
40K	resolve-pkg-maps
40K	buffer-xor
36K	klaw
36K	concat-map
32K	string_decoder
32K	jsonfile
32K	fs.realpath
32K	create-hash
32K	cookie
28K	util-deprecate
28K	ripemd160
28K	randombytes
28K	minimalistic-crypto-utils
28K	create-hmac
28K	base-x
24K	strip-ansi
24K	p-map
24K	md5.js
24K	inherits
24K	indent-string
24K	env-paths
24K	clean-stack
24K	cipher-base
24K	bs58check
24K	brorand
24K	brace-expansion
24K	ansi-regex
24K	aggregate-error
24K	@types/secp256k1
20K	wrappy
20K	universalify
20K	setimmediate
20K	require-from-string
20K	path-parse
20K	path-is-absolute
20K	path-exists
20K	p-try
20K	p-locate
20K	p-limit
20K	os-tmpdir
20K	once
20K	ms
20K	minimalistic-assert
20K	locate-path
20K	inflight
20K	hash-base
20K	find-up
20K	evp_bytestokey
20K	bs58
20K	balanced-match
20K	@types/pbkdf2

@kanej kanej added the no changeset needed This PR doesn't require a changeset label Sep 26, 2024
@kanej kanej force-pushed the feat/skeleton-solidity-test-integration branch from a9a1c5b to a71acb7 Compare September 26, 2024 16:36
@kanej kanej force-pushed the feat/skeleton-solidity-test-integration branch from a71acb7 to 601bbb7 Compare September 26, 2024 17:05
@kanej
feat: add temporary compile task
5a0453b 
This is a hack to leverage the old Hardhat build system from the Hardhat
cli to help with development while the new build system is put in place.
@kanej
feat: add skeleton solidity test plugin
6f036ff 
This is based on the `@ignored/edr` version that comes from the Solidity
Test branch of EDR.

We create a skeleton `solidity-test` plugin, that leverages the
artifacts manager to find contracts and test contracts and pass them off
to the EDR Solidity Test Runner before displaying the results.
@kanej kanej force-pushed the feat/skeleton-solidity-test-integration branch from 601bbb7 to 6f036ff Compare September 26, 2024 17:11
@kanej kanej marked this pull request as ready for review September 26, 2024 18:32
galargh
galargh reviewed Sep 30, 2024
View reviewed changes
Copy link
Member

@galargh galargh left a comment

Choose a reason for hiding this comment

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

I think it'd make sense to merge the skeleton and then iterate on the test runner + replace "v2" placeholders in the future. Before we do that though, I think we should add some inline comments to the files/functions which would make it clear which parts are up for all-out refactor, which are expected to be removed, etc.

Testing

I checked out this branch and successfully run hardhat test:solidity ✅ I quite like test:solidity as a name, btw.

v-next/hardhat/src/internal/builtin-plugins/artifacts/artifacts-manager.ts Show resolved Hide resolved
v-next/hardhat/src/internal/builtin-plugins/artifacts/artifacts-manager.ts Show resolved Hide resolved
v-next/hardhat/src/internal/builtin-plugins/artifacts/artifacts-manager.ts Outdated Show resolved Hide resolved
v-next/hardhat/src/internal/builtin-plugins/artifacts/artifacts-manager.ts Show resolved Hide resolved
v-next/hardhat/src/internal/builtin-plugins/compile/task-action.ts Show resolved Hide resolved
v-next/hardhat-build-system/src/internal/solidity/compiler/compiler-input.ts
language: "Solidity",
sources,
settings,
settings: settings ?? {},
Copy link
Member

Choose a reason for hiding this comment

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

Is there anywhere where we could encounter null-ish settings? Should we check an invariant instead maybe?

v-next/hardhat/src/internal/builtin-plugins/solidity-test/helpers.ts Show resolved Hide resolved
v-next/hardhat/src/internal/builtin-plugins/solidity-test/task-action.ts Show resolved Hide resolved
@kanej
Copy link
Member Author

kanej commented Oct 1, 2024

@galargh I think I caught all of comments.

@kanej kanej requested a review from galargh October 1, 2024 09:35
galargh
galargh approved these changes Oct 1, 2024
View reviewed changes
Copy link
Member

@galargh galargh left a comment

Choose a reason for hiding this comment

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

Thank you 🙇

@kanej kanej removed the request for review from alcuadrado October 1, 2024 20:15
@kanej kanej added this pull request to the merge queue Oct 1, 2024
Merged via the queue into v-next with commit 32b867e Oct 1, 2024
131 checks passed
@kanej kanej deleted the feat/skeleton-solidity-test-integration branch October 1, 2024 20:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@galargh galargh galargh approved these changes

Assignees
No one assigned
Labels
no changeset needed This PR doesn't require a changeset status:ready This issue is ready to be worked on
Projects
Hardhat
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@kanej @galargh