Skip to content

test: cover provable count sum discriminant#747

Closed
thepastaclaw wants to merge 1 commit into
dashpay:developfrom
thepastaclaw:fix-element-discriminant-coverage
Closed

test: cover provable count sum discriminant#747
thepastaclaw wants to merge 1 commit into
dashpay:developfrom
thepastaclaw:fix-element-discriminant-coverage

Conversation

@thepastaclaw

@thepastaclaw thepastaclaw commented May 24, 2026

Copy link
Copy Markdown
Contributor

Test Element Discriminant Coverage

Issue being fixed or feature implemented

Fixes #718.

The Element serialization discriminant drift test covered base discriminants
0..=14, 18, and 19, but omitted ProvableCountProvableSumTree at
discriminant 20.

What was done

  • Added Element::ProvableCountProvableSumTree to
    test_element_serialization_discriminants_match_element_type.
  • Updated the pinned base-variant count from 17 to 18.

How Has This Been Tested

See validation below.

Validation

  • cargo test -p grovedb-element test_element_serialization_discriminants_match_element_type
  • Pre-PR code review gate: ship

Environment: local macOS arm64 workspace.

Breaking Changes

None.

Checklist

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

Summary by CodeRabbit

  • Tests
    • Enhanced test coverage for element serialization by adding support for a new provable tree variant and updating related assertions to validate proper type handling.

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 24, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: cee320e5-8345-465e-a0ed-a03aa93ecaf6

📥 Commits

Reviewing files that changed from the base of the PR and between 9f67e8c and 476f383.

📒 Files selected for processing (1)
  • grovedb-element/src/element_type.rs

📝 Walkthrough

Walkthrough

This PR extends the test_element_serialization_discriminants_match_element_type test to include the ProvableCountProvableSumTree variant (discriminant 20) in its pinned discriminant coverage, updating the base element count assertion from 17 to 18 to reflect the addition.

Changes

Element Discriminant Coverage

Layer / File(s) Summary
Test coverage for ProvableCountProvableSumTree discriminant
grovedb-element/src/element_type.rs
The test_cases vector gains a new test case for discriminant 20 (ProvableCountProvableSumTree), and the assertion verifying the number of base discriminants is updated from 17 to 18.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 A tree so provable, we almost forgot,
Twenty's the discriminant, caught in the spot!
Coverage was missing, the audit did find—
Now all variants tested, with clarity aligned! 🌱

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly references adding test coverage for the ProvableCountProvableSumTree discriminant, which is the core change in the PR.
Linked Issues check ✅ Passed The PR fully addresses issue #718 by adding the omitted ProvableCountProvableSumTree variant to the discriminant coverage test and updating the base-variant count assertion as suggested.
Out of Scope Changes check ✅ Passed All changes are directly scoped to the discriminant coverage test requirement; no unrelated modifications are present.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented May 24, 2026

Copy link
Copy Markdown
Contributor
✅ 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.

@codecov

codecov Bot commented May 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.43%. Comparing base (9f67e8c) to head (476f383).

Additional details and impacted files
@@           Coverage Diff            @@
##           develop     #747   +/-   ##
========================================
  Coverage    91.43%   91.43%           
========================================
  Files          236      236           
  Lines        67111    67116    +5     
========================================
+ Hits         61364    61369    +5     
  Misses        5747     5747           
Components Coverage Δ
grovedb-core 88.94% <ø> (ø)
merk 92.26% <ø> (ø)
storage 86.36% <ø> (ø)
commitment-tree 96.43% <ø> (ø)
mmr 96.79% <ø> (ø)
bulk-append-tree 89.39% <ø> (ø)
element 97.38% <100.00%> (+<0.01%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@QuantumExplorer

Copy link
Copy Markdown
Member

Closing in favor of #742, which addresses the same gap (issue #718: drift-test coverage for Element::ProvableCountProvableSumTree at discriminant 20). The two PRs are functionally duplicates; #742 is the more complete of the pair, so it will be the one merged.

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.

[audit][low] Element discriminant drift test omits ProvableCountProvableSumTree

2 participants