Refactor Test Cases for Better Specification Clarity

sakuyacatcat commented 1 year ago

Overview

We aim to refactor test case names to better describe what specifications they are testing within the code.

Current Situation

For instance, this test case tells us it's testing for a failure in Rsa verification, but it takes time to understand under what circumstances this error occurs.

 function test_FailRsaVerify() public {
        bytes32 INVALID_SHA256_HASHED = hex"47707cfb91cc6bede5f48cde4f1cea391e0ed78338e9240889b045e8808b32d3";
        uint256 ret = INVALID_SHA256_HASHED.pkcs1Sha256Verify(SIGNATURE, EXPONENT, MODULUS);
        assertTrue(ret == 5, "pkcs1Sha256Verify failed");
    }

Desired State

In this particular case, the test is verifying that an error occurs during validation, when checking if the decrypted signature's message part matches the given message's SHA256 hash. It would be more helpful if the test case name directly reflected this.

 function test_FailRsaVerifyByDigestMismatchBetweenSignatureAndGivenHash() public {
        bytes32 INVALID_SHA256_HASHED = hex"47707cfb91cc6bede5f48cde4f1cea391e0ed78338e9240889b045e8808b32d3";
        uint256 ret = INVALID_SHA256_HASHED.pkcs1Sha256Verify(SIGNATURE, EXPONENT, MODULUS);
        assertTrue(ret == 5, "pkcs1Sha256Verify failed");
    }

Benefits

Easier to judge the validity of test cases and notice any missing cases.
Accelerates new developers' understanding of the code, enabling them to contribute to development more quickly.

hiroism007 commented 1 year ago

Thank you for reporting the issue. Would you like to take this on as a ‘good first issue’? Feel free to comment if you have any questions, or you can send smaller PRs if that makes it easier for you.

motemotech commented 1 year ago

@sakuyacatcat Hi, we will organize all myna development in monorepo. Pls work in this repository. https://github.com/MynaWallet/myna-wallet-monorepo

motemotech commented 1 year ago

Also, I migrated to here https://github.com/MynaWallet/myna-wallet-monorepo/issues/16

MynaWallet / contracts