Skip to content

Commit

Permalink
Merge pull request #5496 from NomicFoundation/use-error-assertions
Browse files Browse the repository at this point in the history 
Use `hardhat-test-utils`' functions
  • Loading branch information
@alcuadrado
alcuadrado authored Jul 9, 2024
2 parents 3ca6732 + 250cd29 commit 4c11d36
Show file tree
Hide file tree
Showing 17 changed files with 599 additions and 555 deletions.
34 changes: 32 additions & 2 deletions config-v-next/eslint.cjs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,13 @@ const path = require("path");
* The only packages that should not use this config are our own eslint plugins/rules.
*
* @param {string} configFilePath The path to the config file that is using this function.
* @param {{ onlyHardhatError?: boolean }} options An object with options to enable/disable certain rules.
* @param {{ onlyHardhatError?: boolean, enforceHardhatTestUtils?: boolean }} options An object with options to enable/disable certain rules.
* @returns {import("eslint").Linter.Config}
*/
function createConfig(configFilePath, options = { onlyHardhatError: true }) {
function createConfig(
configFilePath,
options = { onlyHardhatError: true, enforceHardhatTestUtils: true }
) {
/**
* @type {import("eslint").Linter.Config}
*/
Expand Down Expand Up @@ -242,6 +245,18 @@ function createConfig(configFilePath, options = { onlyHardhatError: true }) {
message:
"Top-level await is only allowed in a few cases. Please discuss this change with the team.",
},
{
selector:
"CallExpression[callee.object.name='assert'][callee.property.name=doesNotThrow]",
message:
"Don't use assert.doesNotThrow. Just call the function directly, letting the error bubble up if thrown",
},
{
selector:
"CallExpression[callee.object.name='assert'][callee.property.name=doesNotReject]",
message:
"Don't use assert.doesNotReject. Just await the async-function-call/promise directly, letting the error bubble up if rejected",
},
],
"@typescript-eslint/restrict-plus-operands": "error",
"@typescript-eslint/restrict-template-expressions": [
Expand Down Expand Up @@ -413,6 +428,21 @@ function createConfig(configFilePath, options = { onlyHardhatError: true }) {
});
}

if (options.enforceHardhatTestUtils) {
config.rules["no-restricted-syntax"].push(
{
selector:
"CallExpression[callee.object.name='assert'][callee.property.name=throws]",
message: "Don't use assert.throws. Use our test helpers instead.",
},
{
selector:
"CallExpression[callee.object.name='assert'][callee.property.name=rejects]",
message: "Don't use assert.rejects. Use our test helpers instead.",
}
);
}

return config;
}

Expand Down
3 changes: 3 additions & 0 deletions pnpm-lock.yaml
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions v-next/core/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
"devDependencies": {
"@eslint-community/eslint-plugin-eslint-comments": "^4.3.0",
"@ignored/hardhat-vnext-node-test-reporter": "workspace:^3.0.0-next.2",
"@nomicfoundation/hardhat-test-utils": "workspace:^",
"@microsoft/api-extractor": "^7.43.4",
"@types/node": "^20.14.9",
"@types/semver": "^7.5.8",
Expand Down
20 changes: 12 additions & 8 deletions v-next/core/test/internal/configuration-variables/configuration-variables.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import assert from "node:assert/strict";
import { before, describe, it } from "node:test";

import { HardhatError } from "@ignored/hardhat-vnext-errors";
import { assertRejectsWithHardhatError } from "@nomicfoundation/hardhat-test-utils";

import { ResolvedConfigurationVariableImplementation } from "../../../src/internal/configuration-variables.js";
import { HookManagerImplementation } from "../../../src/internal/hook-manager.js";
Expand Down Expand Up @@ -55,9 +56,10 @@ describe("ResolvedConfigurationVariable", () => {
{ name: "foo", _type: "ConfigurationVariable" },
);

await assert.rejects(
await assertRejectsWithHardhatError(
variable.get(),
new HardhatError(HardhatError.ERRORS.GENERAL.ENV_VAR_NOT_FOUND),
HardhatError.ERRORS.GENERAL.ENV_VAR_NOT_FOUND,
{},
);
});

Expand Down Expand Up @@ -99,11 +101,12 @@ describe("ResolvedConfigurationVariable", () => {

process.env.foo = "not a url";

await assert.rejects(
await assertRejectsWithHardhatError(
variable.getUrl(),
new HardhatError(HardhatError.ERRORS.GENERAL.INVALID_URL, {
HardhatError.ERRORS.GENERAL.INVALID_URL,
{
url: "not a url",
}),
},
);

delete process.env.foo;
Expand All @@ -130,11 +133,12 @@ describe("ResolvedConfigurationVariable", () => {

process.env.foo = "not a bigint";

await assert.rejects(
await assertRejectsWithHardhatError(
variable.getBigInt(),
new HardhatError(HardhatError.ERRORS.GENERAL.INVALID_BIGINT, {
HardhatError.ERRORS.GENERAL.INVALID_BIGINT,
{
value: "not a bigint",
}),
},
);

delete process.env.foo;
Expand Down
67 changes: 37 additions & 30 deletions v-next/core/test/internal/global-options.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import assert from "node:assert/strict";
import { after, before, describe, it } from "node:test";

import { HardhatError } from "@ignored/hardhat-vnext-errors";
import { assertThrowsHardhatError } from "@nomicfoundation/hardhat-test-utils";

import { globalOption, ArgumentType } from "../../src/config.js";
import { RESERVED_ARGUMENT_NAMES } from "../../src/internal/arguments.js";
Expand Down Expand Up @@ -86,7 +87,7 @@ describe("Global Options", () => {
defaultValue: false,
});

assert.throws(
assertThrowsHardhatError(
() =>
buildGlobalOptionDefinitions([
{
Expand All @@ -98,19 +99,18 @@ describe("Global Options", () => {
globalOptions: [globalOptionDefinition2],
},
]),
new HardhatError(
HardhatError.ERRORS.GENERAL.GLOBAL_OPTION_ALREADY_DEFINED,
{
plugin: "plugin2",
globalOption: "globalOption1",
definedByPlugin: "plugin1",
},
),

HardhatError.ERRORS.GENERAL.GLOBAL_OPTION_ALREADY_DEFINED,
{
plugin: "plugin2",
globalOption: "globalOption1",
definedByPlugin: "plugin1",
},
);
});

it("should throw if an option name is not valid", () => {
assert.throws(
assertThrowsHardhatError(
() =>
buildGlobalOptionDefinitions([
{
Expand All @@ -125,15 +125,16 @@ describe("Global Options", () => {
],
},
]),
new HardhatError(HardhatError.ERRORS.ARGUMENTS.INVALID_NAME, {
HardhatError.ERRORS.ARGUMENTS.INVALID_NAME,
{
name: "foo bar",
}),
},
);
});

it("should throw if an option name is reserved", () => {
RESERVED_ARGUMENT_NAMES.forEach((name) => {
assert.throws(
assertThrowsHardhatError(
() =>
buildGlobalOptionDefinitions([
{
Expand All @@ -148,15 +149,16 @@ describe("Global Options", () => {
],
},
]),
new HardhatError(HardhatError.ERRORS.ARGUMENTS.RESERVED_NAME, {
HardhatError.ERRORS.ARGUMENTS.RESERVED_NAME,
{
name,
}),
},
);
});
});

it("should throw if an options default value does not match the type", () => {
assert.throws(
assertThrowsHardhatError(
() =>
buildGlobalOptionDefinitions([
{
Expand All @@ -173,11 +175,12 @@ describe("Global Options", () => {
],
},
]),
new HardhatError(HardhatError.ERRORS.ARGUMENTS.INVALID_VALUE_FOR_TYPE, {
HardhatError.ERRORS.ARGUMENTS.INVALID_VALUE_FOR_TYPE,
{
value: "bar",
name: "defaultValue",
type: ArgumentType.BOOLEAN,
}),
},
);
});
});
Expand Down Expand Up @@ -210,37 +213,39 @@ describe("Global Options", () => {
});

it("should throw if the option name is not valid", () => {
assert.throws(
assertThrowsHardhatError(
() =>
buildGlobalOptionDefinition({
name: "foo bar",
description: "Foo description",
defaultValue: "bar",
}),
new HardhatError(HardhatError.ERRORS.ARGUMENTS.INVALID_NAME, {
HardhatError.ERRORS.ARGUMENTS.INVALID_NAME,
{
name: "foo bar",
}),
},
);
});

it("should throw if the option name is reserved", () => {
RESERVED_ARGUMENT_NAMES.forEach((name) => {
assert.throws(
assertThrowsHardhatError(
() =>
buildGlobalOptionDefinition({
name,
description: "Foo description",
defaultValue: "bar",
}),
new HardhatError(HardhatError.ERRORS.ARGUMENTS.RESERVED_NAME, {
HardhatError.ERRORS.ARGUMENTS.RESERVED_NAME,
{
name,
}),
},
);
});
});

it("should throw if the default value does not match the type", () => {
assert.throws(
assertThrowsHardhatError(
() =>
buildGlobalOptionDefinition({
name: "foo",
Expand All @@ -250,11 +255,12 @@ describe("Global Options", () => {
Intentionally testing an invalid type */
defaultValue: "bar" as any,
}),
new HardhatError(HardhatError.ERRORS.ARGUMENTS.INVALID_VALUE_FOR_TYPE, {
HardhatError.ERRORS.ARGUMENTS.INVALID_VALUE_FOR_TYPE,
{
value: "bar",
name: "defaultValue",
type: ArgumentType.BOOLEAN,
}),
},
);
});
});
Expand Down Expand Up @@ -416,13 +422,14 @@ describe("Global Options", () => {

setEnvVar("HARDHAT_GLOBAL_OPTION1", "not a boolean");

assert.throws(
assertThrowsHardhatError(
() => resolveGlobalOptions({}, globalOptionDefinitions),
new HardhatError(HardhatError.ERRORS.ARGUMENTS.INVALID_VALUE_FOR_TYPE, {
HardhatError.ERRORS.ARGUMENTS.INVALID_VALUE_FOR_TYPE,
{
value: "not a boolean",
name: "globalOption1",
type: ArgumentType.BOOLEAN,
}),
},
);
});
});
Expand Down
53 changes: 27 additions & 26 deletions v-next/core/test/internal/hook-manager.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import assert from "node:assert/strict";
import { describe, it, beforeEach } from "node:test";

import { HardhatError } from "@ignored/hardhat-vnext-errors";
import { ensureError } from "@ignored/hardhat-vnext-utils/error";
import { assertRejectsWithHardhatError } from "@nomicfoundation/hardhat-test-utils";

import { HookManagerImplementation } from "../../src/internal/hook-manager.js";
import { UserInterruptionManagerImplementation } from "../../src/internal/user-interruptions.js";
Expand Down Expand Up @@ -235,20 +237,23 @@ describe("HookManager", () => {

const manager = new HookManagerImplementation([examplePlugin]);

await assert.rejects(
async () =>
manager.runHandlerChain(
"config",
"extendUserConfig",
[{}],
async () => {
return {};
},
),
{
code: "ERR_MODULE_NOT_FOUND",
},
);
try {
await manager.runHandlerChain(
"config",
"extendUserConfig",
[{}],
async () => {
return {};
},
);
} catch (error) {
ensureError(error);
assert.ok("code" in error, "Error has no code property");
assert.equal(error.code, "ERR_MODULE_NOT_FOUND");
return;
}

assert.fail("Expected an error, but none was thrown");
});

it("should throw if a non-filepath is given to the plugin being loaded", async () => {
Expand All @@ -261,7 +266,7 @@ describe("HookManager", () => {

const manager = new HookManagerImplementation([examplePlugin]);

await assert.rejects(
await assertRejectsWithHardhatError(
async () =>
manager.runHandlerChain(
"config",
Expand All @@ -271,14 +276,12 @@ describe("HookManager", () => {
return {};
},
),
new HardhatError(
HardhatError.ERRORS.HOOKS.INVALID_HOOK_FACTORY_PATH,
{
hookCategoryName: "config",
pluginId: "example",
path: "./fixture-plugins/config-plugin.js",
},
),
HardhatError.ERRORS.HOOKS.INVALID_HOOK_FACTORY_PATH,
{
hookCategoryName: "config",
pluginId: "example",
path: "./fixture-plugins/config-plugin.js",
},
);
});
});
Expand Down Expand Up @@ -903,9 +906,7 @@ describe("HookManager", () => {
},
};

assert.doesNotThrow(() =>
hookManager.unregisterHandlers("config", exampleHook),
);
hookManager.unregisterHandlers("config", exampleHook);
});
});
});
Expand Down
Loading

0 comments on commit 4c11d36

Please sign in to comment.