Remove timeout option for solidity tests#8115
Conversation
🦋 Changeset detectedLatest commit: 935a043 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
There was a problem hiding this comment.
Pull request overview
This PR removes the test.solidity.timeout option from Hardhat’s Solidity test configuration, simplifying the API by dropping a misleading “whole-run timeout” that doesn’t behave like a per-test timeout (per issue #7750).
Changes:
- Remove
timeoutfrom Solidity test config types and Zod config schema. - Remove runner/task plumbing that implemented the whole-run timeout behavior.
- Update config validation and config-merge tests to use an alternative config key (
isolate) instead oftimeout.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/hardhat/test/internal/builtin-plugins/test/config.ts | Updates the “preserve existing config” test to use isolate instead of timeout. |
| packages/hardhat/test/internal/builtin-plugins/solidity-test/config-validation.ts | Updates Solidity test config validation tests to validate isolate and its type error message. |
| packages/hardhat/src/internal/builtin-plugins/solidity-test/type-extensions.ts | Removes timeout?: number from the Solidity test config type extensions. |
| packages/hardhat/src/internal/builtin-plugins/solidity-test/task-action.ts | Stops deriving/passing runner timeout options into the Solidity test runner. |
| packages/hardhat/src/internal/builtin-plugins/solidity-test/runner.ts | Removes RunOptions and the timeout timer/error path from the runner implementation. |
| packages/hardhat/src/internal/builtin-plugins/solidity-test/helpers.ts | Removes the config→run-options helper (previously just forwarded config). |
| packages/hardhat/src/internal/builtin-plugins/solidity-test/config.ts | Removes timeout from the Zod schema for test.solidity. |
| packages/hardhat-errors/src/descriptors.ts | Marks the RUNNER_TIMEOUT error descriptor with a deprecation comment. |
Comments suppressed due to low confidence (1)
packages/hardhat/src/internal/builtin-plugins/solidity-test/config.ts:34
- Removing
timeoutfrom the Zod schema here doesn’t actually prevent users from continuing to settest.solidity.timeout: Zod’s defaultz.objectbehavior won’t error on unknown keys, andresolveSolidityTestUserConfiglater spreads...userConfig.test?.solidityinto the resolved config (sohre.config.test.solidity.timeoutcan still exist at runtime, even though it no longer does anything). To fully remove the option (per the PR goal), either explicitly omittimeoutduring config resolution, or make the relevant schema strict and surface a clear config validation error whentimeoutis present.
const solidityTestUserConfigType = z.object({
fsPermissions: z
.object({
readWriteFile: z.array(z.string()).optional(),
readFile: z.array(z.string()).optional(),
| RUNNER_TIMEOUT: { | ||
| // [DEPRECATED] | ||
| number: 801, | ||
| messageTemplate: `Runner timed out after {duration} ms. |
There was a problem hiding this comment.
Now that the runner timeout option is removed and RUNNER_TIMEOUT appears to be unreachable, consider either removing this descriptor entry (if error codes aren’t part of a stable external API) or updating the actual message/website description to indicate deprecation. A standalone // [DEPRECATED] comment is easy to miss and isn’t consistent with most of the descriptors file.
There was a problem hiding this comment.
Not sure on how to deprecate a DESCRIPTOR, do we have a standard process?
There was a problem hiding this comment.
We can't remove them, because this generates the website, and older versions link to it.
I think what you did here makes sense.
| testFunctionOverrides?: TestFunctionOverride[]; | ||
| } | ||
|
|
||
| export function solidityTestConfigToRunOptions( |
There was a problem hiding this comment.
used only to get the timeout, so now it is no longer needed
Fixes: #7750
PR to update website docs: NomicFoundation/hardhat-website#250