Skip to content

Comments

test(dpp): add StepDecreasingAmount and validate_update keyword/description error path tests#3125

Open
thepastaclaw wants to merge 3 commits intodashpay:v3.1-devfrom
thepastaclaw:test/validation-error-paths-contract
Open

test(dpp): add StepDecreasingAmount and validate_update keyword/description error path tests#3125
thepastaclaw wants to merge 3 commits intodashpay:v3.1-devfrom
thepastaclaw:test/validation-error-paths-contract

Conversation

@thepastaclaw
Copy link
Contributor

@thepastaclaw thepastaclaw commented Feb 20, 2026

Issue Being Fixed

Tracker: thepastaclaw/tracker#14 (parent: #11)

Addresses findings C-13 and I-02 from the rs-dpp test quality audit.

What was done

C-13: StepDecreasingAmount validation — 3 tests → 11 tests

Added 8 new error-path tests covering all previously untested branches:

  • test_step_decreasing_amount_invalid_zero_distribution_start — distribution_start_amount=0
  • test_step_decreasing_amount_invalid_max_distribution_start — distribution_start_amount > MAX_DISTRIBUTION_PARAM
  • test_step_decreasing_amount_invalid_trailing_exceeds_start — trailing > start amount
  • test_step_decreasing_amount_invalid_max_interval_count_too_low — max_interval_count=1 (< 2)
  • test_step_decreasing_amount_invalid_max_interval_count_too_high — max_interval_count=1025 (> 1024)
  • test_step_decreasing_amount_invalid_zero_numerator — decrease_per_interval_numerator=0
  • test_step_decreasing_amount_invalid_numerator_gte_denominator — numerator >= denominator
  • test_step_decreasing_amount_invalid_min_value_exceeds_start — min_value > distribution_start_amount

I-02: validate_update keyword/description validation — 0 tests → 7 tests

Added tests for all 5 keyword error paths plus 2 description error paths:

  • should_return_invalid_result_if_too_many_keywords — 51 keywords → TooManyKeywordsError
  • should_return_invalid_result_if_keyword_too_short — 2-char keyword → InvalidKeywordLengthError
  • should_return_invalid_result_if_keyword_too_long — 51-char keyword → InvalidKeywordLengthError
  • should_return_invalid_result_if_keyword_has_invalid_chars — whitespace in keyword → InvalidKeywordCharacterError
  • should_return_invalid_result_if_duplicate_keywords — same keyword twice → DuplicateKeywordsError
  • should_return_invalid_result_if_description_too_short — 2-char description → InvalidDescriptionLengthError
  • should_return_invalid_result_if_description_too_long — 101-char description → InvalidDescriptionLengthError

All tests compile and pass (56 targeted tests, 0 failures).

Summary by CodeRabbit

  • Tests
    • Added comprehensive test coverage for distribution function validation scenarios, including boundary conditions and parameter validation checks.
    • Expanded validation tests for data contract update operations, covering keyword and description validation requirements.

Validation

  1. What was tested

    • CI checks on this PR (Rust packages linting, tests, builds)
  2. Results

    • All Rust CI checks passing
  3. Environment

    • GitHub Actions CI for dashpay/platform

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 20, 2026

📝 Walkthrough

Walkthrough

This PR adds comprehensive unit tests for two validation functions in rs-dpp, covering previously untested error branches. Tests are added for StepDecreasingAmount distribution validation (covering invalid parameter combinations) and validate_update_v0 keyword and description validation (covering invalid lengths, characters, and duplicates).

Changes

Cohort / File(s) Summary
Distribution Function Validation Tests
packages/rs-dpp/src/data_contract/associated_token/token_perpetual_distribution/distribution_function/validation.rs
Adds 8+ unit tests for DistributionFunction::StepDecreasingAmount covering invalid parameter scenarios including zero/max distribution start, trailing exceeds start, interval count bounds, zero/excessive numerator, and min value checks.
Contract Update Validation Tests
packages/rs-dpp/src/data_contract/methods/validate_update/v0/mod.rs
Adds 7 unit tests for validate_update_v0 covering invalid keywords (too many, incorrect length, invalid characters, duplicates) and invalid descriptions (too short, too long), each asserting the appropriate error variant.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 Test branches bloom where shadows hid,
Each validation path now bid,
Step-decreasing, keywords bright,
Coverage spreads—bugs take flight!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main changes: adding StepDecreasingAmount and validate_update keyword/description error path tests.
Linked Issues check ✅ Passed The PR fully addresses both C-13 and I-02 requirements from issue #14: adds 8 StepDecreasingAmount error tests and 7 validate_update keyword/description tests, matching all specified invalid scenarios.
Out of Scope Changes check ✅ Passed All changes are in-scope: the PR adds only unit tests for two specific validators (StepDecreasingAmount and validate_update) with no unrelated code modifications.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@thepastaclaw
Copy link
Contributor Author

@coderabbitai review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 20, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@thepastaclaw
Copy link
Contributor Author

@coderabbitai review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 21, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@thepastaclaw thepastaclaw force-pushed the test/validation-error-paths-contract branch from b25e974 to dfeb089 Compare February 21, 2026 18:02
thepastaclaw added a commit to thepastaclaw/platform that referenced this pull request Feb 21, 2026
@thepastaclaw thepastaclaw force-pushed the test/validation-error-paths-contract branch from dfeb089 to 15f4bdb Compare February 22, 2026 04:44
@github-actions github-actions bot added this to the v3.1.0 milestone Feb 22, 2026
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
packages/rs-dpp/src/data_contract/methods/validate_update/v0/mod.rs (1)

615-802: New keyword/description validation tests look correct and well-structured.

All seven tests properly set up the fixture, bump the version, mutate the target field to an invalid value, and assert the expected error variant. The error paths exercised align with the validation logic at lines 270-323.

One minor observation: existing tests in this file (e.g., lines 386-391, 415-416) use assert_matches! with if guards to verify error details (contract ID, keyword value, etc.), whereas these new tests only match the variant with a wildcard _. Adding guards would strengthen the assertions—for example, verifying e.data_contract_id() == old_data_contract.id() on the keyword errors—but this is optional.

,

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/rs-dpp/src/data_contract/methods/validate_update/v0/mod.rs` around
lines 615 - 802, Update the new tests to assert error details instead of using
wildcards: for each test that currently matches e.g.
ConsensusError::BasicError(BasicError::TooManyKeywordsError(_)) or
BasicError::InvalidKeywordLengthError(_), change the pattern to bind the inner
error (e.g. BasicError::TooManyKeywordsError(e)) and add an if guard that checks
properties like e.data_contract_id() == old_data_contract.id() and, where
relevant, e.keyword() or e.keyword_value() equals the invalid value used in the
test; locate these changes in the tests named
should_return_invalid_result_if_too_many_keywords, _if_keyword_too_short,
_if_keyword_too_long, _if_keyword_has_invalid_chars, _if_duplicate_keywords,
_if_description_too_short, and _if_description_too_long, and update the
assert_matches! patterns to include the bound variable and guards to validate
error details.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@packages/rs-dpp/src/data_contract/methods/validate_update/v0/mod.rs`:
- Around line 615-802: Update the new tests to assert error details instead of
using wildcards: for each test that currently matches e.g.
ConsensusError::BasicError(BasicError::TooManyKeywordsError(_)) or
BasicError::InvalidKeywordLengthError(_), change the pattern to bind the inner
error (e.g. BasicError::TooManyKeywordsError(e)) and add an if guard that checks
properties like e.data_contract_id() == old_data_contract.id() and, where
relevant, e.keyword() or e.keyword_value() equals the invalid value used in the
test; locate these changes in the tests named
should_return_invalid_result_if_too_many_keywords, _if_keyword_too_short,
_if_keyword_too_long, _if_keyword_has_invalid_chars, _if_duplicate_keywords,
_if_description_too_short, and _if_description_too_long, and update the
assert_matches! patterns to include the bound variable and guards to validate
error details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant