diff --git a/v-next/example-project/hardhat.config.ts b/v-next/example-project/hardhat.config.ts index 2ef3a7ad41..a71a2e2bbc 100644 --- a/v-next/example-project/hardhat.config.ts +++ b/v-next/example-project/hardhat.config.ts @@ -163,6 +163,7 @@ const config: HardhatUserConfig = { tests: { mocha: "test/mocha", nodeTest: "test/node", + solidity: "test/contracts", }, }, solidity: { diff --git a/v-next/example-project/contracts/WithForge.t.sol b/v-next/example-project/test/contracts/WithForge.sol similarity index 100% rename from v-next/example-project/contracts/WithForge.t.sol rename to v-next/example-project/test/contracts/WithForge.sol diff --git a/v-next/hardhat/src/internal/builtin-plugins/solidity-test/helpers.ts b/v-next/hardhat/src/internal/builtin-plugins/solidity-test/helpers.ts index a5f71e45ce..2866988aca 100644 --- a/v-next/hardhat/src/internal/builtin-plugins/solidity-test/helpers.ts +++ b/v-next/hardhat/src/internal/builtin-plugins/solidity-test/helpers.ts @@ -1,66 +1,25 @@ -import type { ArtifactsManager } from "../../../types/artifacts.js"; -import type { Artifact } from "@ignored/edr"; - -import { HardhatError } from "@ignored/hardhat-vnext-errors"; -import { exists } from "@ignored/hardhat-vnext-utils/fs"; -import { resolveFromRoot } from "@ignored/hardhat-vnext-utils/path"; - -export async function getArtifacts( - hardhatArtifacts: ArtifactsManager, -): Promise { - const fqns = await hardhatArtifacts.getAllFullyQualifiedNames(); - const artifacts: Artifact[] = []; - - for (const fqn of fqns) { - const hardhatArtifact = await hardhatArtifacts.readArtifact(fqn); - const buildInfo = await hardhatArtifacts.getBuildInfo(fqn); - - if (buildInfo === undefined) { - throw new HardhatError( - HardhatError.ERRORS.SOLIDITY_TESTS.BUILD_INFO_NOT_FOUND_FOR_CONTRACT, - { - fqn, - }, - ); - } - - const id = { - name: hardhatArtifact.contractName, - solcVersion: buildInfo.solcVersion, - source: hardhatArtifact.sourceName, - }; - - const contract = { - abi: JSON.stringify(hardhatArtifact.abi), - bytecode: hardhatArtifact.bytecode, - deployedBytecode: hardhatArtifact.deployedBytecode, - }; - - const artifact = { id, contract }; - artifacts.push(artifact); - } - - return artifacts; -} - -export async function isTestArtifact( - root: string, - artifact: Artifact, -): Promise { - const { source } = artifact.id; - - if (!source.endsWith(".t.sol")) { - return false; - } - - // NOTE: We also check whether the file exists in the workspace to filter out - // the artifacts from node modules. - const sourcePath = resolveFromRoot(root, source); - const sourceExists = await exists(sourcePath); - - if (!sourceExists) { - return false; - } - - return true; +import type { + ArtifactId as EdrArtifactId, + Artifact as EdrArtifact, +} from "@ignored/edr"; + +import path from "node:path"; + +/** + * This function returns the test suite ids associated with the given artifacts. + * The test suite ID is the relative path of the test file, relative to the + * project root. + */ +export async function getTestSuiteIds( + artifacts: EdrArtifact[], + rootTestFilePaths: string[], + projectRoot: string, +): Promise { + const testSources = rootTestFilePaths.map((p) => + path.relative(projectRoot, p), + ); + + return artifacts + .map(({ id }) => id) + .filter(({ source }) => testSources.includes(source)); } diff --git a/v-next/hardhat/src/internal/builtin-plugins/solidity-test/hook-handlers/config.ts b/v-next/hardhat/src/internal/builtin-plugins/solidity-test/hook-handlers/config.ts new file mode 100644 index 0000000000..293b2efa9a --- /dev/null +++ b/v-next/hardhat/src/internal/builtin-plugins/solidity-test/hook-handlers/config.ts @@ -0,0 +1,61 @@ +import type { ConfigHooks } from "@ignored/hardhat-vnext/types/hooks"; + +import { isObject } from "@ignored/hardhat-vnext-utils/lang"; +import { resolveFromRoot } from "@ignored/hardhat-vnext-utils/path"; +import { + conditionalUnionType, + validateUserConfigZodType, +} from "@ignored/hardhat-vnext-zod-utils"; +import { z } from "zod"; + +const userConfigType = z.object({ + paths: z + .object({ + test: conditionalUnionType( + [ + [isObject, z.object({ solidity: z.string().optional() })], + [(data) => typeof data === "string", z.string()], + ], + "Expected a string or an object with an optional 'solidity' property", + ).optional(), + }) + .optional(), +}); + +export default async (): Promise> => { + const handlers: Partial = { + validateUserConfig: async (userConfig) => { + return validateUserConfigZodType(userConfig, userConfigType); + }, + resolveUserConfig: async ( + userConfig, + resolveConfigurationVariable, + next, + ) => { + const resolvedConfig = await next( + userConfig, + resolveConfigurationVariable, + ); + + let testsPath = userConfig.paths?.tests; + + // TODO: use isObject when the type narrowing issue is fixed + testsPath = + typeof testsPath === "object" ? testsPath.solidity : testsPath; + testsPath ??= "test"; + + return { + ...resolvedConfig, + paths: { + ...resolvedConfig.paths, + tests: { + ...resolvedConfig.paths.tests, + solidity: resolveFromRoot(resolvedConfig.paths.root, testsPath), + }, + }, + }; + }, + }; + + return handlers; +}; diff --git a/v-next/hardhat/src/internal/builtin-plugins/solidity-test/index.ts b/v-next/hardhat/src/internal/builtin-plugins/solidity-test/index.ts index 8211f9fbdc..e3f63852fc 100644 --- a/v-next/hardhat/src/internal/builtin-plugins/solidity-test/index.ts +++ b/v-next/hardhat/src/internal/builtin-plugins/solidity-test/index.ts @@ -3,8 +3,13 @@ import type { HardhatPlugin } from "../../../types/plugins.js"; import { ArgumentType } from "../../../types/arguments.js"; import { task } from "../../core/config.js"; +import "./type-extensions.js"; + const hardhatPlugin: HardhatPlugin = { id: "builtin:solidity-tests", + hookHandlers: { + config: import.meta.resolve("./hook-handlers/config.js"), + }, tasks: [ task(["test", "solidity"], "Run the Solidity tests") .setAction(import.meta.resolve("./task-action.js")) @@ -16,8 +21,12 @@ const hardhatPlugin: HardhatPlugin = { defaultValue: 60 * 60 * 1000, }) .addFlag({ - name: "noCompile", - description: "Don't compile the project before running the tests", + name: "force", + description: "Force compilation even if no files have changed", + }) + .addFlag({ + name: "quiet", + description: "Makes the compilation process less verbose", }) .build(), ], diff --git a/v-next/hardhat/src/internal/builtin-plugins/solidity-test/task-action.ts b/v-next/hardhat/src/internal/builtin-plugins/solidity-test/task-action.ts index 02b8e14019..987e87a577 100644 --- a/v-next/hardhat/src/internal/builtin-plugins/solidity-test/task-action.ts +++ b/v-next/hardhat/src/internal/builtin-plugins/solidity-test/task-action.ts @@ -1,43 +1,68 @@ import type { RunOptions } from "./runner.js"; import type { TestEvent } from "./types.js"; +import type { BuildOptions } from "../../../types/solidity/build-system.js"; import type { NewTaskActionFunction } from "../../../types/tasks.js"; import { finished } from "node:stream/promises"; +import { getAllFilesMatching } from "@ignored/hardhat-vnext-utils/fs"; import { createNonClosingWriter } from "@ignored/hardhat-vnext-utils/stream"; -import { getArtifacts, isTestArtifact } from "./helpers.js"; +import { shouldMergeCompilationJobs } from "../solidity/build-profiles.js"; +import { + getArtifacts, + throwIfSolidityBuildFailed, +} from "../solidity/build-results.js"; + +import { getTestSuiteIds } from "./helpers.js"; import { testReporter } from "./reporter.js"; import { run } from "./runner.js"; interface TestActionArguments { timeout: number; - noCompile: boolean; + force: boolean; + quiet: boolean; } const runSolidityTests: NewTaskActionFunction = async ( - { timeout, noCompile }, + { timeout, force, quiet }, hre, ) => { - if (!noCompile) { - await hre.tasks.getTask("compile").run({}); - console.log(); - } - - const artifacts = await getArtifacts(hre.artifacts); - const testSuiteIds = ( - await Promise.all( - artifacts.map(async (artifact) => { - if (await isTestArtifact(hre.config.paths.root, artifact)) { - return artifact.id; - } + const rootSourceFilePaths = await hre.solidity.getRootFilePaths(); + // NOTE: A test file is either a file with a `.sol` extension in the `tests.solidity` + // directory or a file with a `.t.sol` extension in the `sources.solidity` directory + const rootTestFilePaths = ( + await Promise.all([ + getAllFilesMatching(hre.config.paths.tests.solidity, (f) => + f.endsWith(".sol"), + ), + ...hre.config.paths.sources.solidity.map(async (dir) => { + return getAllFilesMatching(dir, (f) => f.endsWith(".t.sol")); }), - ) - ).filter((artifact) => artifact !== undefined); + ]) + ).flat(1); + + const buildOptions: BuildOptions = { + force, + buildProfile: hre.globalOptions.buildProfile, + mergeCompilationJobs: shouldMergeCompilationJobs( + hre.globalOptions.buildProfile, + ), + quiet, + }; - if (testSuiteIds.length === 0) { - return; - } + // NOTE: We compile all the sources together with the tests + const rootFilePaths = [...rootSourceFilePaths, ...rootTestFilePaths]; + const results = await hre.solidity.build(rootFilePaths, buildOptions); + + throwIfSolidityBuildFailed(results); + + const artifacts = await getArtifacts(results, hre.config.paths.artifacts); + const testSuiteIds = await getTestSuiteIds( + artifacts, + rootTestFilePaths, + hre.config.paths.root, + ); console.log("Running Solidity tests"); console.log(); diff --git a/v-next/hardhat/src/internal/builtin-plugins/solidity-test/type-extensions.ts b/v-next/hardhat/src/internal/builtin-plugins/solidity-test/type-extensions.ts new file mode 100644 index 0000000000..85abcead6d --- /dev/null +++ b/v-next/hardhat/src/internal/builtin-plugins/solidity-test/type-extensions.ts @@ -0,0 +1,11 @@ +import "@ignored/hardhat-vnext/types/config"; + +declare module "@ignored/hardhat-vnext/types/config" { + export interface TestPathsUserConfig { + solidity?: string; + } + + export interface TestPathsConfig { + solidity: string; + } +} diff --git a/v-next/hardhat/src/internal/builtin-plugins/solidity/build-results.ts b/v-next/hardhat/src/internal/builtin-plugins/solidity/build-results.ts new file mode 100644 index 0000000000..40520c4ba8 --- /dev/null +++ b/v-next/hardhat/src/internal/builtin-plugins/solidity/build-results.ts @@ -0,0 +1,95 @@ +import type { + Artifact as HardhatArtifact, + BuildInfo, +} from "../../../types/artifacts.js"; +import type { Artifact as EdrArtifact } from "@ignored/edr"; + +import path from "node:path"; + +import { HardhatError } from "@ignored/hardhat-vnext-errors"; +import { readJsonFile } from "@ignored/hardhat-vnext-utils/fs"; + +import { + FileBuildResultType, + type CompilationJobCreationError, + type FailedFileBuildResult, + type FileBuildResult, +} from "../../../types/solidity.js"; + +type SolidityBuildResults = + | Map + | CompilationJobCreationError; +type SuccessfulSolidityBuildResults = Map< + string, + Exclude +>; + +/** + * This function asserts that the given Solidity build results are successful. + * It throws a HardhatError if the build results indicate that the compilation + * job failed. + */ +export function throwIfSolidityBuildFailed( + results: SolidityBuildResults, +): asserts results is SuccessfulSolidityBuildResults { + if ("reason" in results) { + throw new HardhatError( + HardhatError.ERRORS.SOLIDITY.COMPILATION_JOB_CREATION_ERROR, + { + reason: results.formattedReason, + rootFilePath: results.rootFilePath, + buildProfile: results.buildProfile, + }, + ); + } + + const sucessful = [...results.values()].every( + ({ type }) => + type === FileBuildResultType.CACHE_HIT || + type === FileBuildResultType.BUILD_SUCCESS, + ); + + if (!sucessful) { + throw new HardhatError(HardhatError.ERRORS.SOLIDITY.BUILD_FAILED); + } +} + +/** + * This function returns the artifacts generated during the compilation associated + * with the given Solidity build results. It relies on the fact that each successful + * build result has a corresponding artifact generated property. + */ +export async function getArtifacts( + results: SuccessfulSolidityBuildResults, + artifactsRootPath: string, +): Promise { + const artifacts: EdrArtifact[] = []; + + for (const [source, result] of results.entries()) { + for (const artifactPath of result.contractArtifactsGenerated) { + const artifact: HardhatArtifact = await readJsonFile(artifactPath); + const buildInfo: BuildInfo = await readJsonFile( + path.join(artifactsRootPath, "build-info", `${result.buildId}.json`), + ); + + const id = { + name: artifact.contractName, + solcVersion: buildInfo.solcVersion, + source, + }; + + const contract = { + abi: JSON.stringify(artifact.abi), + bytecode: artifact.bytecode, + deployedBytecode: artifact.deployedBytecode, + }; + + artifacts.push({ + id, + contract, + }); + } + } + + return artifacts; +} diff --git a/v-next/hardhat/src/internal/builtin-plugins/solidity/build-system/build-system.ts b/v-next/hardhat/src/internal/builtin-plugins/solidity/build-system/build-system.ts index 349a39a8db..cf5acc396e 100644 --- a/v-next/hardhat/src/internal/builtin-plugins/solidity/build-system/build-system.ts +++ b/v-next/hardhat/src/internal/builtin-plugins/solidity/build-system/build-system.ts @@ -67,7 +67,7 @@ export interface SolidityBuildSystemOptions { export class SolidityBuildSystemImplementation implements SolidityBuildSystem { readonly #options: SolidityBuildSystemOptions; - readonly #defaultConcurrenty = Math.max(os.cpus().length - 1, 1); + readonly #defaultConcurrency = Math.max(os.cpus().length - 1, 1); #downloadedCompilers = false; constructor(options: SolidityBuildSystemOptions) { @@ -78,7 +78,10 @@ export class SolidityBuildSystemImplementation implements SolidityBuildSystem { const localFilesToCompile = ( await Promise.all( this.#options.soliditySourcesPaths.map((dir) => - getAllFilesMatching(dir, (f) => f.endsWith(".sol")), + getAllFilesMatching( + dir, + (f) => f.endsWith(".sol") && !f.endsWith(".t.sol"), + ), ), ) ).flat(1); @@ -118,7 +121,7 @@ export class SolidityBuildSystemImplementation implements SolidityBuildSystem { compilationJobs, (compilationJob) => this.runCompilationJob(compilationJob), { - concurrency: options?.concurrency ?? this.#defaultConcurrenty, + concurrency: options?.concurrency ?? this.#defaultConcurrency, // An error when running the compiler is not a compilation failure, but // a fatal failure trying to run it, so we just throw on the first error stopOnError: true, diff --git a/v-next/hardhat/src/internal/builtin-plugins/solidity/tasks/compile.ts b/v-next/hardhat/src/internal/builtin-plugins/solidity/tasks/compile.ts index 0bb657db05..b4e341f25d 100644 --- a/v-next/hardhat/src/internal/builtin-plugins/solidity/tasks/compile.ts +++ b/v-next/hardhat/src/internal/builtin-plugins/solidity/tasks/compile.ts @@ -1,10 +1,9 @@ import type { NewTaskActionFunction } from "../../../../types/tasks.js"; -import { HardhatError } from "@ignored/hardhat-vnext-errors"; import { resolveFromRoot } from "@ignored/hardhat-vnext-utils/path"; -import { FileBuildResultType } from "../../../../types/solidity.js"; import { shouldMergeCompilationJobs } from "../build-profiles.js"; +import { throwIfSolidityBuildFailed } from "../build-results.js"; import { isNpmRootPath } from "../build-system/root-paths-utils.js"; interface CompileActionArguments { @@ -37,26 +36,7 @@ const compileAction: NewTaskActionFunction = async ( quiet, }); - if ("reason" in results) { - throw new HardhatError( - HardhatError.ERRORS.SOLIDITY.COMPILATION_JOB_CREATION_ERROR, - { - reason: results.formattedReason, - rootFilePath: results.rootFilePath, - buildProfile: results.buildProfile, - }, - ); - } - - const sucessful = [...results.values()].every( - ({ type }) => - type === FileBuildResultType.CACHE_HIT || - type === FileBuildResultType.BUILD_SUCCESS, - ); - - if (!sucessful) { - throw new HardhatError(HardhatError.ERRORS.SOLIDITY.BUILD_FAILED); - } + throwIfSolidityBuildFailed(results); // If we recompiled the entire project we cleanup the artifacts if (files.length === 0) { diff --git a/v-next/hardhat/src/types/solidity/build-system.ts b/v-next/hardhat/src/types/solidity/build-system.ts index 821622e61e..f10aaa5b4f 100644 --- a/v-next/hardhat/src/types/solidity/build-system.ts +++ b/v-next/hardhat/src/types/solidity/build-system.ts @@ -149,7 +149,11 @@ export interface CacheHitFileBuildResult { type: FileBuildResultType.CACHE_HIT; // TODO: Should we remove this? It is a buildId of an already existing build // info. + // NOTE: The buildId and contractArtifactsGenerated are useful when one uses + // the build system programatically and wants to find out what artifacts + // were generated for a given file, and with what configuration. buildId: string; + contractArtifactsGenerated: string[]; } export interface SuccessfulFileBuildResult { @@ -265,7 +269,7 @@ export interface SolidityBuildSystem { * This method should only be used after a complete build has succeeded, as * it relies on the build system to have generated all the necessary artifact * files. - + * @param rootFilePaths All the root files of the project. */ cleanupArtifacts(rootFilePaths: string[]): Promise; diff --git a/v-next/hardhat/test/internal/builtin-plugins/solidity-test/helpers.ts b/v-next/hardhat/test/internal/builtin-plugins/solidity-test/helpers.ts deleted file mode 100644 index 245bf895ec..0000000000 --- a/v-next/hardhat/test/internal/builtin-plugins/solidity-test/helpers.ts +++ /dev/null @@ -1,98 +0,0 @@ -import type { Artifact } from "@ignored/edr"; - -import assert from "node:assert/strict"; -import { describe, it } from "node:test"; - -import { isTestArtifact } from "../../../../src/internal/builtin-plugins/solidity-test/helpers.js"; - -const testCases = [ - { - contract: "Abstract", - expected: true, - }, - { - contract: "NoTest", - expected: true, - }, - { - contract: "PublicTest", - expected: true, - }, - { - contract: "ExternalTest", - expected: true, - }, - { - contract: "PrivateTest", - expected: true, - }, - { - contract: "InternalTest", - expected: true, - }, - { - contract: "PublicInvariant", - expected: true, - }, - { - contract: "ExternalInvariant", - expected: true, - }, - { - contract: "PrivateInvariant", - expected: true, - }, - { - contract: "InternalInvariant", - expected: true, - }, -]; - -describe("isTestArtifact", () => { - for (const { contract, expected } of testCases) { - it(`should return ${expected} for the ${contract} contract`, async () => { - const artifact: Artifact = { - id: { - name: contract, - source: `test-fixtures/Test.t.sol`, - solcVersion: "0.8.20", - }, - contract: { - abi: "", - }, - }; - const actual = await isTestArtifact(import.meta.dirname, artifact); - assert.equal(actual, expected); - }); - } - - it("should return false if a file does not exist", async () => { - const artifact: Artifact = { - id: { - name: "Contract", - source: `test-fixtures/NonExistent.t.sol`, - solcVersion: "0.8.20", - }, - contract: { - abi: "", - }, - }; - const actual = await isTestArtifact(import.meta.dirname, artifact); - assert.equal(actual, false); - }); - - it("should return false if the file has the wrong extension", async () => { - const artifact: Artifact = { - id: { - name: "Contract", - source: `test-fixtures/WrongExtension.sol`, - solcVersion: "0.8.20", - }, - contract: { - abi: "", - }, - }; - const actual = await isTestArtifact(import.meta.dirname, artifact); - assert.equal(actual, false); - }); -}); diff --git a/v-next/hardhat/test/internal/builtin-plugins/solidity-test/test-fixtures/Test.t.sol b/v-next/hardhat/test/internal/builtin-plugins/solidity-test/test-fixtures/Test.t.sol deleted file mode 100644 index 025f6a7e35..0000000000 --- a/v-next/hardhat/test/internal/builtin-plugins/solidity-test/test-fixtures/Test.t.sol +++ /dev/null @@ -1,43 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity 0.8.20; - -abstract contract Abstract { - function testPublic() public {} - function testExternal() external {} - function invariantPublic() public {} - function invariantExternal() external {} -} - -contract NoTest {} - -contract PublicTest { - function test() public {} -} - -contract ExternalTest { - function test() external {} -} - -contract PrivateTest { - function test() private {} -} - -contract InternalTest { - function test() internal {} -} - -contract PublicInvariant { - function invariant() public {} -} - -contract ExternalInvariant { - function invariant() external {} -} - -contract PrivateInvariant { - function invariant() private {} -} - -contract InternalInvariant { - function invariant() internal {} -} diff --git a/v-next/hardhat/test/internal/core/config-validation.ts b/v-next/hardhat/test/internal/core/config-validation.ts index 27849516bd..551af370db 100644 --- a/v-next/hardhat/test/internal/core/config-validation.ts +++ b/v-next/hardhat/test/internal/core/config-validation.ts @@ -1,5 +1,8 @@ import type { HardhatUserConfig } from "../../../src/config.js"; -import type { ProjectPathsUserConfig } from "../../../src/types/config.js"; +import type { + ProjectPathsUserConfig, + TestPathsUserConfig, +} from "../../../src/types/config.js"; import type { HardhatPlugin } from "../../../src/types/plugins.js"; import type { EmptyTaskDefinition, @@ -1247,7 +1250,8 @@ describe("config validation", function () { it("should work when the tests and sources properties are both objects", async function () { // Objects are not validated because they are customizable by the user const paths: ProjectPathsUserConfig = { - tests: { randomProperty: "randomValue" }, + /* eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- testing validations for js users who can bypass type checks */ + tests: { randomProperty: "randomValue" } as TestPathsUserConfig, sources: {}, }; diff --git a/v-next/hardhat/test/internal/core/hook-manager.ts b/v-next/hardhat/test/internal/core/hook-manager.ts index 8c4940e4f9..7b57b3ade1 100644 --- a/v-next/hardhat/test/internal/core/hook-manager.ts +++ b/v-next/hardhat/test/internal/core/hook-manager.ts @@ -257,8 +257,9 @@ describe("HookManager", () => { _context: HookContext, givenHre: HardhatRuntimeEnvironment, ): Promise => { - givenHre.config.paths.tests = - "./test-folder-from-plugin1"; + givenHre.config.paths.tests = { + solidity: "./test-folder-from-plugin1", + }; }, } as Partial; @@ -277,8 +278,9 @@ describe("HookManager", () => { _context: HookContext, givenHre: HardhatRuntimeEnvironment, ): Promise => { - givenHre.config.paths.tests = - "./test-folder-from-overriding-plugin2"; + givenHre.config.paths.tests = { + solidity: "./test-folder-from-overriding-plugin2", + }; }, } as Partial; @@ -302,7 +304,7 @@ describe("HookManager", () => { assert.equal(result.length, 2); assert.equal( - hre.config.paths.tests, + hre.config.paths.tests.solidity, "./test-folder-from-overriding-plugin2", ); });