-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat: add fallible methods to artifact manager #8095
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -75,6 +75,27 @@ export interface ArtifactManager { | |
| contractNameOrFullyQualifiedName: ContractNameT, | ||
| ): Promise<GetArtifactByName<ContractNameT>>; | ||
|
|
||
| /** | ||
| * Tries to read an artifact, returning `undefined` if it doesn't exist. | ||
| * | ||
| * Use this instead of `readArtifact` if you want to avoid constructing an error when the artifact doesn't exist, which can be expensive if it happens often. | ||
| * | ||
| * @param contractNameOrFullyQualifiedName The name of the contract. | ||
| * It can be a contract bare contract name (e.g. "Token") if it's | ||
|
Comment on lines
+83
to
+84
|
||
| * unique in your project, or a fully qualified contract name | ||
| * (e.g. "contract/token.sol:Token") otherwise. TypeScript's language server | ||
| * autocompletes the names of the contracts that have already been built. If | ||
| * your contract name isn't in the list, you can still use it, and/or run | ||
| * `hardhat build` to get it in the list. | ||
| * @throws Throws an error if a non-unique contract name is used, | ||
| * indicating which fully qualified names can be used instead. | ||
| */ | ||
| tryToReadArtifact< | ||
| ContractNameT extends StringWithArtifactContractNamesAutocompletion, | ||
| >( | ||
| contractNameOrFullyQualifiedName: ContractNameT, | ||
| ): Promise<GetArtifactByName<ContractNameT> | undefined>; | ||
|
Comment on lines
+93
to
+97
|
||
|
|
||
| /** | ||
| * Returns the absolute path to the given artifact. | ||
| * | ||
|
|
@@ -86,6 +107,20 @@ export interface ArtifactManager { | |
| */ | ||
| getArtifactPath(contractNameOrFullyQualifiedName: string): Promise<string>; | ||
|
|
||
| /** | ||
| * Tries to get the absolute path to the given artifact, returning `undefined` if it doesn't exist. | ||
| * | ||
| * Use this instead of `getArtifactPath` if you want to avoid constructing an error when the artifact doesn't exist, which can be expensive if it happens often. | ||
| * | ||
| * @param contractNameOrFullyQualifiedName The name or fully qualified name | ||
| * of the contract. | ||
| * @throws Throws an error if a non-unique contract name is used, | ||
| * indicating which fully qualified names can be used instead. | ||
| */ | ||
| tryToGetArtifactPath( | ||
| contractNameOrFullyQualifiedName: string, | ||
| ): Promise<string | undefined>; | ||
|
Comment on lines
+120
to
+122
|
||
|
|
||
| /** | ||
| * Returns true if an artifact exists. | ||
| * | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -40,6 +40,23 @@ export class MockArtifactManager implements ArtifactManager { | |||||||||||||||||||||||||||||||||||||
| return artifact as GetArtifactByName<ContractNameT>; | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| public async tryToReadArtifact< | ||||||||||||||||||||||||||||||||||||||
| ContractNameT extends StringWithArtifactContractNamesAutocompletion, | ||||||||||||||||||||||||||||||||||||||
| >( | ||||||||||||||||||||||||||||||||||||||
| contractNameOrFullyQualifiedName: ContractNameT, | ||||||||||||||||||||||||||||||||||||||
| ): Promise<GetArtifactByName<ContractNameT> | undefined> { | ||||||||||||||||||||||||||||||||||||||
| const artifact = this.#artifacts.get(contractNameOrFullyQualifiedName); | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| if (artifact === undefined) { | ||||||||||||||||||||||||||||||||||||||
| return undefined; | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| /* eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- | ||||||||||||||||||||||||||||||||||||||
| We are asserting that the artifact is of the correct type, which won't be | ||||||||||||||||||||||||||||||||||||||
| really used during tests. */ | ||||||||||||||||||||||||||||||||||||||
| return artifact as GetArtifactByName<ContractNameT>; | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| public async getArtifactPath( | ||||||||||||||||||||||||||||||||||||||
| _contractNameOrFullyQualifiedName: string, | ||||||||||||||||||||||||||||||||||||||
| ): Promise<string> { | ||||||||||||||||||||||||||||||||||||||
|
|
@@ -51,6 +68,17 @@ export class MockArtifactManager implements ArtifactManager { | |||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| public async tryToGetArtifactPath( | ||||||||||||||||||||||||||||||||||||||
| _contractNameOrFullyQualifiedName: string, | ||||||||||||||||||||||||||||||||||||||
| ): Promise<string | undefined> { | ||||||||||||||||||||||||||||||||||||||
| throw new HardhatError( | ||||||||||||||||||||||||||||||||||||||
| HardhatError.ERRORS.CORE.INTERNAL.NOT_IMPLEMENTED_ERROR, | ||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||
| message: "Not implemented in MockArtifactManager", | ||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+72
to
+79
|
||||||||||||||||||||||||||||||||||||||
| _contractNameOrFullyQualifiedName: string, | |
| ): Promise<string | undefined> { | |
| throw new HardhatError( | |
| HardhatError.ERRORS.CORE.INTERNAL.NOT_IMPLEMENTED_ERROR, | |
| { | |
| message: "Not implemented in MockArtifactManager", | |
| }, | |
| ); | |
| contractNameOrFullyQualifiedName: string, | |
| ): Promise<string | undefined> { | |
| const artifact = this.#artifacts.get(contractNameOrFullyQualifiedName); | |
| if (artifact === undefined) { | |
| return undefined; | |
| } | |
| // Return a deterministic fake path for tests when the artifact exists. | |
| return `${contractNameOrFullyQualifiedName}.json`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same issue as the Hardhat test mock: a
tryTo*API should not throw for the normal 'not found' flow. In this mock, consider mappingcontractNameOrFullyQualifiedNameto the existing#artifactsPathsentries and returningundefinedwhen absent, to better reflect the production contract and avoid test brittleness.