Description
Multiple checked-in #[test] functions consist entirely of comments describing intended
behavior, with no actual contract invocation and no assert!/assert_eq! — meaning they
currently report as "passing" while verifying nothing. In contracts/tests/financing_pool_edge_cases.rs
alone:
test_yield_calculation_with_equal_positions (lines 83-102)
test_yield_calculation_with_unequal_positions (lines 104-115)
test_yield_calculation_with_small_position (lines 117-128)
test_yield_calculation_with_large_position (lines 130-138)
test_repayment_lock_prevents_concurrent_repay (lines 197-204)
test_repayment_lock_cleared_on_success (lines 206-212)
test_repayment_lock_cleared_on_error (lines 214-220)
test_repay_exceeds_face_value_allowed (lines 296-302)
test_repay_already_repaid_fails (lines 317-323)
test_repayment_completes_when_fully_funded (lines 325-332)
test_record_position_atomicity (lines 245-266, calls try_record_position but asserts nothing on the result)
test_repaid_amount_arithmetic_overflow (lines 388-396)
In contracts/tests/access_control_edge_cases.rs:
test_cannot_transfer_admin_to_zero_address (lines 183-191)
test_transfer_admin_to_existing_admin_allowed (lines 247-254)
That is 14 confirmed empty/assertion-free tests, all describing genuinely important invariants
(yield-distribution precision, repayment-lock cleanup, over-repayment handling, zero-address
admin-transfer rejection) that are currently not verified at all despite appearing in the
test suite as named, "passing" tests.
Requirements and Context
This is distinct from "add more tests" filler: these tests already exist, are already counted
in CI's green checkmark, and describe exactly what they're supposed to verify in their own
doc comments — they just never call the contract. A reviewer or new contributor skimming test
names would reasonably (and incorrectly) conclude these invariants are covered. Implementing
real bodies requires wiring up actual release_funds/record_position/repay sequences with
real Stellar Asset Contract tokens, mirroring the pattern already proven in
contracts/tests/src/lib.rs's test_multi_investor_partial_recovery_default_end_to_end.
Suggested Execution
git checkout -b fix/edge-case-tests-empty-bodies
- For each of the 12 empty tests in
financing_pool_edge_cases.rs, implement a real sequence:
deploy a mock token, call release_funds/record_position to establish pool state, then
exercise the specific edge case (equal/unequal/small/large positions; lock-cleared-on-success
vs. on-error by forcing one call to fail; over-repayment; double-repay rejection; full
automatic closure) and assert on the resulting Pool/Position/token-balance state.
- For the 2 empty tests in
access_control_edge_cases.rs, either implement a genuine
zero-address rejection test (Soroban's Address doesn't have a trivial "zero" literal in
tests, so this may require documenting why it's not directly testable and converting the
test into a targeted unit test of the underlying validation helper instead), and implement
real assertions for the idempotent-transfer-to-existing-admin case.
- Where an edge case turns out to be genuinely untestable in the current test harness (e.g.
true concurrent reentrancy under Soroban's synchronous model), replace the placeholder with
an explicit #[ignore] plus a doc comment explaining why, rather than leaving a silently
passing empty test.
- Run
make fmt and make test.
Acceptance Criteria
Guidelines: PR description must include Closes #<issue-number>.
Complexity: High (200 points)
Description
Multiple checked-in
#[test]functions consist entirely of comments describing intendedbehavior, with no actual contract invocation and no
assert!/assert_eq!— meaning theycurrently report as "passing" while verifying nothing. In
contracts/tests/financing_pool_edge_cases.rsalone:
test_yield_calculation_with_equal_positions(lines 83-102)test_yield_calculation_with_unequal_positions(lines 104-115)test_yield_calculation_with_small_position(lines 117-128)test_yield_calculation_with_large_position(lines 130-138)test_repayment_lock_prevents_concurrent_repay(lines 197-204)test_repayment_lock_cleared_on_success(lines 206-212)test_repayment_lock_cleared_on_error(lines 214-220)test_repay_exceeds_face_value_allowed(lines 296-302)test_repay_already_repaid_fails(lines 317-323)test_repayment_completes_when_fully_funded(lines 325-332)test_record_position_atomicity(lines 245-266, callstry_record_positionbut asserts nothing on the result)test_repaid_amount_arithmetic_overflow(lines 388-396)In
contracts/tests/access_control_edge_cases.rs:test_cannot_transfer_admin_to_zero_address(lines 183-191)test_transfer_admin_to_existing_admin_allowed(lines 247-254)That is 14 confirmed empty/assertion-free tests, all describing genuinely important invariants
(yield-distribution precision, repayment-lock cleanup, over-repayment handling, zero-address
admin-transfer rejection) that are currently not verified at all despite appearing in the
test suite as named, "passing" tests.
Requirements and Context
This is distinct from "add more tests" filler: these tests already exist, are already counted
in CI's green checkmark, and describe exactly what they're supposed to verify in their own
doc comments — they just never call the contract. A reviewer or new contributor skimming test
names would reasonably (and incorrectly) conclude these invariants are covered. Implementing
real bodies requires wiring up actual
release_funds/record_position/repaysequences withreal Stellar Asset Contract tokens, mirroring the pattern already proven in
contracts/tests/src/lib.rs'stest_multi_investor_partial_recovery_default_end_to_end.Suggested Execution
git checkout -b fix/edge-case-tests-empty-bodiesfinancing_pool_edge_cases.rs, implement a real sequence:deploy a mock token, call
release_funds/record_positionto establish pool state, thenexercise the specific edge case (equal/unequal/small/large positions; lock-cleared-on-success
vs. on-error by forcing one call to fail; over-repayment; double-repay rejection; full
automatic closure) and assert on the resulting
Pool/Position/token-balance state.access_control_edge_cases.rs, either implement a genuinezero-address rejection test (Soroban's
Addressdoesn't have a trivial "zero" literal intests, so this may require documenting why it's not directly testable and converting the
test into a targeted unit test of the underlying validation helper instead), and implement
real assertions for the idempotent-transfer-to-existing-admin case.
true concurrent reentrancy under Soroban's synchronous model), replace the placeholder with
an explicit
#[ignore]plus a doc comment explaining why, rather than leaving a silentlypassing empty test.
make fmtandmake test.Acceptance Criteria
financing_pool_edge_cases.rscontain real contract calls and at least one meaningful assertionaccess_control_edge_cases.rscontain real assertions or are explicitly marked#[ignore]with a documented reason#[test]function in either file consists solely of commentscargo test -p kora-testsshows an increased number of meaningfully-asserting tests, verifiable by diffGuidelines: PR description must include
Closes #<issue-number>.Complexity: High (200 points)