Skip to content

Commit

Permalink
test(ethereum): fix broken validations in tests
Browse files Browse the repository at this point in the history
Primary Changes
---------------
1. Added code to deployContract in plugin-ledger-connector-ethereum.ts that checks if any
unexpected arguments are present, and if so throws an error.
2. Modified test on line 169 in geth-invoke-web3-method-v1.test.ts so that required argument for
invokeRawWeb3EthMethod is missing as required by the test description.

Fixes #3487

Signed-off-by: ashnashahgrover <ashnashahgrover777@gmail.com>
  • Loading branch information
ashnashahgrover committed Sep 21, 2024
1 parent 438a800 commit b216088
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 72 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -943,6 +943,22 @@ export class PluginLedgerConnectorEthereum
): Promise<RunTransactionResponse> {
Checks.truthy(req, "deployContract() request arg");

// Validate the keys in the request object
const validKeys = [
"web3SigningCredential",
"contract",
"constructorArgs",
"gasConfig",
"value",
];
const extraKeys = Object.keys(req).filter(
(key) => !validKeys.includes(key),
);

if (extraKeys.length > 0) {
throw new Error(`Invalid parameters: ${extraKeys.join(", ")}`);
}

if (isWeb3SigningCredentialNone(req.web3SigningCredential)) {
throw new Error(`Cannot deploy contract with pre-signed TX`);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -249,29 +249,6 @@ describe("Ethereum contract deploy and invoke using keychain tests", () => {
});

test("deployContract with additional parameters should fail", async () => {
// this try-catch statement was not refactored because calling deployContract with additional parameters is actually not
// causing an error.

// try {
// await apiClient.deployContract({
// contract: {
// contractJSON: HelloWorldContractJson,
// },
// web3SigningCredential: {
// ethAccount: WHALE_ACCOUNT_ADDRESS,
// secret: "",
// type: Web3SigningCredentialType.GethKeychainPassword,
// },
// gas: 1000000,
// fake: 4,
// } as DeployContractV1Request);
// //test is failing because "fail" is not defined. Without the fail statement, the test actually passes.
// fail("Expected deployContract call to fail but it succeeded.");
// } catch (error) {
// console.log("deployContract failed as expected");
// }

// have the left the original assertion above as a comment for additional context, this can be removed once this test is debugged.
await expect(
apiClient.deployContract({
contract: {
Expand All @@ -284,11 +261,10 @@ describe("Ethereum contract deploy and invoke using keychain tests", () => {
},
gas: 1000000,
fake: 4,
} as DeployContractV1Request)
} as DeployContractV1Request),
).rejects.toThrow();

log.info("deployContract failed as expected");

});

//////////////////////////////////
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -253,32 +253,6 @@ describe("Ethereum contract deploy and invoke using keychain tests", () => {
});

test("deployContract with additional parameters should fail", async () => {
// did not refactor because the test is not actually failing
// it returns a message saying: INFO (PluginLedgerConnectorEthereum): Contract deployed successfully, saving address in keychain entry
// it only hits the catch statement because "fail is not defined"

// try {
// await apiClient.deployContract({
// contract: {
// contractName: HelloWorldContractJson.contractName,
// keychainId: keychainPlugin.getKeychainId(),
// },
// web3SigningCredential: {
// ethAccount: WHALE_ACCOUNT_ADDRESS,
// secret: "",
// type: Web3SigningCredentialType.GethKeychainPassword,
// },
// gas: 1000000,
// fake: 4,
// } as DeployContractV1Request);
// fail("Expected deployContract call to fail but it succeeded.");
// } catch (error) {
// log.info("error message:");
// log.info(error.message);
// log.info("deployContract failed as expected");
// }

// have the left the original assertion above as a comment for additional context, this can be removed once this test is debugged.
await expect(
apiClient.deployContract({
contract: {
Expand All @@ -292,11 +266,10 @@ describe("Ethereum contract deploy and invoke using keychain tests", () => {
},
gas: 1000000,
fake: 4,
} as DeployContractV1Request)
} as DeployContractV1Request),
).rejects.toThrow();

log.info("deployContract failed as expected");

log.info("deployContract failed as expected");
});

//////////////////////////////////
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,26 +167,14 @@ describe("invokeRawWeb3EthMethod Tests", () => {
});

test("invokeRawWeb3EthMethod with missing arg throws error (getBlock)", async () => {
// did not refactor because the test is not failing.
// try {
// const connectorResponse = connector.invokeRawWeb3EthMethod({
// methodName: "getBlock",
// });

// await connectorResponse;
// //This test is actually passing, but the statement below is not being printed.
// fail("Calling getBlock with missing argument should throw an error");
// } catch (err) {
// expect(err).toBeTruthy();
// }

// have the left the original assertion above as a comment for additional context, this can be removed once this test is debugged.
// Should "missing arg" mean no method name is provided? Because the only required parameter is methodName
// Have taken out methodName from the invocation of the method as I'm guessing that is what this test if supposed to check for?
// It will also fail if methodName is an empty string.
// Or should we just delete this test ?
await expect(
connector.invokeRawWeb3EthMethod({
methodName: "getBlock",
})
// @ts-expect-error: the script fails otherwise
connector.invokeRawWeb3EthMethod(),
).rejects.toBeTruthy();

});

test("invokeRawWeb3EthMethod with invalid arg throws error (getBlock)", async () => {
Expand Down

0 comments on commit b216088

Please sign in to comment.