Repository navigation
feat(solidity-tests): scope inline config directives by test profile - #1715
Conversation
🦋 Changeset detectedLatest commit: 3a489b4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1715 +/- ##
==========================================
+ Coverage 80.41% 80.51% +0.09%
==========================================
Files 464 466 +2
Lines 81719 82198 +479
Branches 81719 82198 +479
==========================================
+ Hits 65714 66181 +467
- Misses 13763 13766 +3
- Partials 2242 2251 +9 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Adds profile-aware Solidity inline configuration across the parser, runner, and N-API.
Changes:
- Adds selected/declared profile configuration and scoped directive precedence.
- Reports undeclared profiles through renamed structured errors.
- Adds Rust and JavaScript integration coverage.
Reviewed changes
Copilot reviewed 19 out of 20 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
js/integration-tests/solidity-tests/test/unit.ts |
Tests undeclared-profile errors. |
js/integration-tests/solidity-tests/test/fuzz.ts |
Tests profile-specific fuzz runs. |
js/integration-tests/solidity-tests/test-contracts/InlineConfigUndeclaredProfile.t.sol |
Adds invalid-profile fixture. |
js/integration-tests/solidity-tests/test-contracts/InlineConfigProfiles.t.sol |
Adds profile precedence fixture. |
crates/edr_solidity_tests/tests/testdata/default/fuzz/FuzzProfileOverride.t.sol |
Adds Rust profile fixture. |
crates/edr_solidity_tests/tests/it/helpers.rs |
Declares fixture profiles. |
crates/edr_solidity_tests/tests/it/fuzz.rs |
Tests profile resolution. |
crates/edr_solidity_tests/src/multi_runner.rs |
Passes profiles into collection. |
crates/edr_solidity_tests/src/inline_config/provider.rs |
Propagates profile context. |
crates/edr_solidity_tests/src/inline_config/profiles.rs |
Defines profile configuration. |
crates/edr_solidity_tests/src/inline_config/overrides.rs |
Passes profiles to directives. |
crates/edr_solidity_tests/src/inline_config/error.rs |
Adds profile errors. |
crates/edr_solidity_tests/src/inline_config/directives.rs |
Implements profile scoping and precedence. |
crates/edr_solidity_tests/src/inline_config.rs |
Exports and documents profiles. |
crates/edr_solidity_tests/src/config.rs |
Adds profiles to runner configuration. |
crates/edr_napi/src/solidity_tests/inline_config.rs |
Exposes renamed structured error. |
crates/edr_napi/src/solidity_tests/config.rs |
Accepts profile arguments. |
crates/edr_napi/index.d.ts |
Updates TypeScript declarations. |
crates/edr_napi_core/src/solidity/config.rs |
Validates and converts profile configuration. |
.changeset/olive-pugs-shave.md |
Documents feature and breaking change. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
3745265 to
67ea335
Compare
acd109d to
6ed5af5
Compare
Wodann
left a comment
There was a problem hiding this comment.
The main concern I have is around documenting the product decisions. We should do so both at the top-level inline_config.rs's module-level doc comments and on the Hardhat 3 website. Even more so, since we deviate from Foundry.
Ideally, this is written by a human rather than Claude and remains concise, merely stating the rules.
E.g.
/// Directives are handled in this order of priority:
/// 1. Function-level > Contract-level
/// 2. Profile > bare
///
/// Custom profiles do not inherit from the "default" profile.
| let DirectiveTarget::Function(function) = target else { | ||
| return Ok(()); | ||
| }; | ||
| let is_fuzz_test = function.starts_with("test"); |
There was a problem hiding this comment.
question: Is "test" the identifier for a fuzz test?
There was a problem hiding this comment.
Partially - fuzz tests have start with "test" (same as unit tests) AND have at least one parameter. The current implementation accepts both and doesn't report failures when using fuzz configs for unit tests, they are simply ignored (same as Foundry).
However, I believe that we should be more restrictive. IIRC in the initial implementation I made, data about parameters was not immediately available, which is why I made this choice. But it is now trivial to obtain from the Slang compilation unit ast and reject fuzz configs in unit tests. I'll address this in a follow-up PR since it's not related to this change. Great catch, thanks!
Adds test profile support to inline configuration, so Hardhat can offer profiles for Solidity tests.
SolidityTestRunnerConfigArgsgains two optional fields,testProfileanddeclaredTestProfiles, both defaulting todefault. A caller that passes neither is unaffected.A directive's key can now carry a profile prefix:
Where both set the same key, the prefixed one wins. Prefixes are validated against the declared profiles rather than the selected one, so a typo fails whichever profile is running.
Behaviour changes
default.-prefixed directive is now scoped to thedefaultprofile instead of applying unconditionally.InlineConfigUnsupportedProfileis renamed toInlineConfigUndeclaredProfileand now carriesdeclaredProfiles.The design doc has the full rationale, including why prefixes validate against the declared set and why directives for unselected profiles are still validated.
Hardhat PR: NomicFoundation/hardhat#8590