line coverage for cosign-v1 - #8207
Kassaking7 wants to merge 7 commits into
Conversation
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
| // outside of a dry run. Coverage is measured in Debug where the assert | ||
| // above aborts first, so the body of this defensive branch is | ||
| // unreachable there. |
There was a problem hiding this comment.
we dont need to comment about the unreachable here
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
| // Only reachable if the on-ledger SignerList is corrupt | ||
| // (SignerListSet re-runs the same deserialize on write). | ||
| // LCOV_EXCL_START | ||
| JLOG(ctx.j.fatal()) << "TransactionProposalCreate: unparseable SignerList: " | ||
| << transToken(accountSigners.error()); | ||
| return std::unexpected(tefBAD_LEDGER); | ||
| // LCOV_EXCL_STOP |
There was a problem hiding this comment.
Can we test this by modifying the OpenLedger reference (similar to the examples in Invariants test) ? I vaguely remember seeing some corrupt / unreadable SignerList unit-tests. In such a corrupt ledger-state, it is useful to validate that we hit the tefBAD_LEDGER error code.
| return tefINTERNAL; // LCOV_EXCL_LINE | ||
| isSigner = isAuthorizedFor(delegateAccount); | ||
| if (!isSigner) | ||
| return isSigner.error(); |
There was a problem hiding this comment.
Could you explain why this line is unreachable in unit-tests? My understanding is that we need to construct a corrupt ledger-state where the Delegate ledger-object exists, however the associated SignerList is unparseable.
| catch (UnknownTxnType const& e) | ||
| { | ||
| // Should never happen | ||
| // LCOV_EXCL_START | ||
| JLOG(debugLog().fatal()) << "Unknown transaction type in invokeCheckPermission: " | ||
| << e.txnType; | ||
| UNREACHABLE("xrpl::invokeCheckPermission : unknown transaction type"); | ||
| return temUNKNOWN; |
There was a problem hiding this comment.
Isn't this block testable by specifying a sfDelegate inside a proposed-transaction and ensuring that the proposed-transaction has an ill-specified transaction-type ??
| [[maybe_unused]] unsigned const apiVersion) | ||
| { | ||
| if (!params.isObject()) | ||
| return parseObjectID(params, fieldName, "hex string"); |
There was a problem hiding this comment.
An error string to the effect of "expect a hex string to parse the proposal-ID" would be helpful here.
5 participants
High Level Overview of Change
Context of Change
API Impact
libxrplchange (any change that may affectlibxrplor dependents oflibxrpl)