-
-
Notifications
You must be signed in to change notification settings - Fork 6
Remove hardhat and replace with vitest #27
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
Conversation
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
All alerts resolved. Learn more about Socket for GitHub. This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. Ignoring alerts on: |
@SocketSecurity ignore npm/[email protected] |
5184882
to
e1e119b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need this file with a reference to hardhat?
- standardise vitest dependency across monorepo. - update packages to 'type':'module' to fix vitest configuration resolution. - fix exports in @metamask/delegation-core to correctly target commonjs and esm.
e1e119b
to
fb342c2
Compare
packages/delegation-toolkit/test/caveatBuilder/createCaveatBuilder.test.ts
Show resolved
Hide resolved
const factory = '0xfD9261637e09DD3c58643cDecC64A75EA235bBe8'; | ||
const implementations = { | ||
HybridDeleGatorImpl: '0x4Ec9f3fDaBCA7b4588f6C0bA94464De12Bbd2E22', | ||
MultiSigDeleGatorImpl: '0xa8F73EC8374b6fF40712AF6C21D1e79e1a5186aa', | ||
} as const; | ||
const deploySalt = | ||
'0x11a8b2a8c7cf03cd3ef899b0e934306bff980bdb10a1d9e84c7598253fee9bcf'; | ||
const ownerAddress = '0x72ec0f88409247ebf9dc04a3521e5162d90cab1e'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if these hardcoded addresses could give us false positives in the future, meaning that they seem to work but maybe only for an old version
📝 Description
Removes hardhat and related dependencies. Adds
vitest
as test runner forpackages/delegation-toolkit
.Previously
packages/delegation-toolkit
tests were run with hardhat.Coverage report is now shown when
yarn:test
is executed.🚀 Why?
Test runners are inconsistent throughout the monorepo -
vitest
,hardhat
. Additionallyanvil
andhardhat-network
are used in different packages for network simulation.Hardhat
introduces a complex dependency graph, including some critical dependencies. Removing this, and depending onvitest
for these unit tests brings consistency, and reduces dependency management.Additionally this simplifies our unit-testing, forcing us to isolate the component under test, while we can continue to do integration testing with our e2e test suite.
Resolves the following vulnerabilities:
🧪 How to Test?
This should bring no breaking changes, but improves the developer experience when running unit tests in
/packages/delegation-toolkit
.Because this impacts how @metamask/delegation-core and @metamask/delegation-toolkit are bundled, it's important to test both esm and commonjs consumers for both packages.
I have validated this with commonjs with nodejs directly:
ensuring that each is correctly resolved.
I have validated with esm with the following:
List any breaking changes:
📋 Checklist
Check off completed items:
🔗 Related Issues
Link to related issues:
Closes #
Related to #
📚 Additional Notes
Any additional information, concerns, or context: