feat: return typed Result from test runner and telemetry task actions#8015
feat: return typed Result from test runner and telemetry task actions#8015
Conversation
🦋 Changeset detectedLatest commit: ed00780 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 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
Migrates built-in test runners (solidity, nodejs, mocha) and the telemetry task action to return the new typed Result (from #8012) instead of mutating process.exitCode, and introduces a shared TestSummary type so the parent test task can aggregate results via type guards.
Changes:
- Add shared
TestSummarytype and switch test runner task actions to returnResult<TestSummary, TestSummary>. - Update the parent
testtask to aggregate subtask results usingisResult+ type guards and return aResultitself. - Update related tests, peer bump metadata, and add a changeset documenting the new public types/helpers.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| v-next/hardhat/src/types/test.ts | Introduces the shared TestSummary type for test runner payloads. |
| v-next/hardhat/src/internal/builtin-plugins/test/task-action.ts | Aggregates subtask results using Result and returns a Result from the parent test task. |
| v-next/hardhat/src/internal/builtin-plugins/solidity-test/task-action.ts | Returns Result<TestSummary, TestSummary> instead of setting process.exitCode. |
| v-next/hardhat/src/internal/builtin-plugins/telemetry/task-action.ts | Returns Result instead of setting process.exitCode for invalid flag combinations. |
| v-next/hardhat/test/internal/builtin-plugins/solidity-test/task-action.ts | Updates tests to assert on Result.success rather than process.exitCode. |
| v-next/hardhat-node-test-runner/src/task-action.ts | Returns Result<TestSummary, TestSummary> and uses successResult/errorResult. |
| v-next/hardhat-node-test-runner/test/index.ts | Removes process.exitCode save/restore since runner no longer sets it. |
| v-next/hardhat-mocha/src/task-action.ts | Returns Result<TestSummary, TestSummary> and uses successResult/errorResult. |
| v-next/hardhat-mocha/test/index.ts | Updates expectations to match the new Result wrapper shape. |
| v-next/hardhat-mocha/test/env.ts | Removes process.exitCode save/restore since runner no longer sets it. |
| .peer-bumps.json | Updates peer bump reasons to include new Result helpers and TestSummary type usage. |
| .changeset/ninety-dolls-laugh.md | Documents the new Result return pattern and TestSummary interface as a patch release. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
7730a39 to
82600c4
Compare
alcuadrado
left a comment
There was a problem hiding this comment.
I think there are two small improvements that could be made, but in general looks good
Co-authored-by: Patricio Palladino <email@patriciopalladino.com>
dc9cabb to
d24c904
Compare
| return isObject(value); | ||
| // Old plugins may only return { failed, passed } without skipped/todo, | ||
| // so we accept a partial shape and fill defaults in the coordinator. | ||
| interface PartialTestSummary extends Omit<TestSummary, "skipped" | "todo"> { |
There was a problem hiding this comment.
Older versions of hardhat-mocha didn't include skipped and todo in TestSummary. To keep these versions compatible, we make the fields optional and default them on lines 125-129:
testSummaries[summaryId] = {
skipped: 0,
todo: 0,
...summary,
};
Update test runner (
solidity,node,mocha) andtelemetrytask actions to returnResultinstead of settingprocess.exitCode. Define a sharedTestSummarytype and useisResultwith type guards in the parent test task to aggregate results without type assertions.This should be reviewed after #8012.