refactor: migrate all tests to Bulloak BTT format (#63) by RonTuretzky · Pull Request #119 · BreadchainCoop/crowdstake.fun

RonTuretzky · 2026-04-15T23:08:57Z

Summary

Add .tree BTT specification files for all 13 test contracts
Restructure test function names to follow Branching Tree Technique conventions
Split compound tests into finer-grained BTT functions (103 → 115 test functions)
Zero logic changes — only naming and organization updated

Closes #63

Test plan

All 115 tests pass (same 4 fork-dependent pre-existing failures)
bulloak scaffold generates matching structure for all .tree files
No test coverage reduction

Stack: PR 2 of 10 (0.0.1) — stacked on #118

Copilot

Pull request overview

Refactors the Forge test suite to align with Bulloak’s Branching Tree Technique (BTT) by introducing .tree specs and reorganizing/renaming tests to match the intended hierarchical structure.

Changes:

Added BTT .tree specification files for existing test contracts.
Renamed and reorganized test functions into BTT-style “when/given/should” naming and sections.
Split some previously compound tests into more granular cases and repositioned helper functions for readability.

Reviewed changes

Copilot reviewed 26 out of 26 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
test/automation/AutomationBase.tree	Adds BTT tree spec for automation tests.
test/automation/AutomationBase.t.sol	Splits/renames automation tests to BTT-style cases.
test/VotingStreakNFTModule.tree	Adds BTT tree spec for VotingStreakNFTModule tests.
test/VotingStreakNFTModule.t.sol	Renames/resections tests to match BTT structure.
test/VotingRecipientRegistry.tree	Adds BTT tree spec for VotingRecipientRegistry tests.
test/VotingRecipientRegistry.t.sol	Renames tests and reorganizes into BTT-style groupings.
test/VotingModuleSimple.tree	Adds BTT tree spec for VotingModuleSimple tests.
test/VotingModuleSimple.t.sol	Renames tests and moves vote-digest helper earlier.
test/VotingModule.tree	Adds BTT tree spec for VotingModule tests.
test/VotingModule.t.sol	Renames tests and moves vote-digest helper earlier.
test/TimeWeightedVotingPower.tree	Adds BTT tree spec for TimeWeightedVotingPower tests.
test/TimeWeightedVotingPower.t.sol	Renames/resections TWVP tests to align with tree nodes.
test/RecipientRegistry.tree	Adds BTT tree spec for RecipientRegistry tests.
test/RecipientRegistry.t.sol	Restructures RecipientRegistry tests into BTT “when/given” layout.
test/MockDistributionManager.tree	Adds BTT tree spec for MockDistributionManager tests.
test/MockDistributionManager.t.sol	Splits/renames tests into BTT-style cases.
test/FactoryModuleDeployment.tree	Adds BTT tree spec for factory deployment tests.
test/FactoryModuleDeployment.t.sol	Renames/resections factory deployment tests to BTT-style names.
test/DistributionCycleIntegration.tree	Adds BTT tree spec for integration test.
test/DistributionCycleIntegration.t.sol	Renames integration tests into BTT-style cases.
test/CycleModule.tree	Adds BTT tree spec for CycleModule tests.
test/CycleModule.t.sol	Renames/resections CycleModule tests to BTT conventions.
test/BreadKitTest.tree	Adds BTT tree spec for BreadKit tests.
test/BreadKitTest.t.sol	Renames/resections BreadKit tests into BTT-style cases.
test/AdminRecipientRegistry.tree	Adds BTT tree spec for AdminRecipientRegistry tests.
test/AdminRecipientRegistry.t.sol	Renames/resections AdminRecipientRegistry tests to BTT-style cases.

Comments suppressed due to low confidence (1)

test/VotingStreakNFTModule.t.sol:233

test_RevertWhen_NonOwnerSetsNFTContract asserts both the non-owner revert case and the owner success path, but the corresponding .tree leaf only describes the revert behavior. For clearer BTT mapping (and easier failure attribution), consider splitting this into two tests or updating the tree to include the owner-success scenario.

    function test_RevertWhen_NonOwnerSetsNFTContract() public {
        // Arrange
        address newNftAddress = address(0x9999);

        // Act & Assert - Non-owner should revert
        vm.prank(nonOwner);
        vm.expectRevert();
        harness.setNFTContract(newNftAddress);

        // Act & Assert - Owner should succeed
        vm.prank(admin);
        harness.setNFTContract(newNftAddress);

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-16T01:06:07Z

+    function test_WhenANon_recipientProposes() external {
+        // it should revert with NotARecipient
        vm.prank(NON_RECIPIENT);
        vm.expectRevert(VotingRecipientRegistry.NotARecipient.selector);
        registry.proposeAddition(NEW_RECIPIENT);
    }

-    function test_RevertOnNonRecipientVote() public {
+    function test_WhenANon_recipientVotes() external {
+        // it should revert with NotEligibleVoter


Function names test_WhenANon_recipientProposes / test_WhenANon_recipientVotes include an underscore inside the word, which is inconsistent with the rest of the BTT-style names and reduces readability/searchability. Rename to use a single token (e.g., NonRecipient) while keeping the same meaning.

Copilot · 2026-04-16T01:06:07Z

    }

-    function test_RevertOnRemovingNonExistent() public {
+    function test_WhenRemovingNon_existentRecipient() external {


test_WhenRemovingNon_existentRecipient uses an underscore inside “Non_existent”, which is inconsistent with other test names and makes grepping/filtering harder. Consider renaming to NonExistentRecipient (or similar) for readability.

Suggested change

function test_WhenRemovingNon_existentRecipient() external {

function test_WhenRemovingNonExistentRecipient() external {

Copilot · 2026-04-16T01:06:07Z

    }

-    function test_RevertOnRemovingNonExistent() public {
+    function test_RevertWhen_RemovingANon_existentRecipient() external {


test_RevertWhen_RemovingANon_existentRecipient includes an underscore inside “Non_existent”, which is inconsistent with other test names and reduces readability. Consider renaming to NonExistentRecipient (or similar).

Suggested change

function test_RevertWhen_RemovingANon_existentRecipient() external {

function test_RevertWhen_RemovingANonExistentRecipient() external {

Copilot · 2026-04-16T01:06:08Z

+    function test_RevertWhen_ANon_adminQueuesAddition() external {
+        // it should revert
        vm.prank(address(0xdead));
        vm.expectRevert();
        registry.queueRecipientAddition(RECIPIENT_1);
    }

-    function test_OnlyAdminCanQueueRemoval() public {
+    function test_RevertWhen_ANon_adminQueuesRemoval() external {
+        // it should revert


test_RevertWhen_ANon_adminQueuesAddition / test_RevertWhen_ANon_adminQueuesRemoval include an underscore inside “Non_admin”. For consistency with the rest of the BTT-style names, consider renaming to NonAdmin... (no underscore inside the token).

Copilot · 2026-04-16T01:06:08Z

+    function test_WhenProposingAnAddition() external {
+        // it should create proposal with auto-vote from proposer
+        // it should snapshot required votes at creation
        vm.prank(RECIPIENT_1);


test_WhenProposingAnAddition validates two separate behaviors (auto-vote + requiredVotes snapshot) while test/VotingRecipientRegistry.tree lists them as two distinct leaves. This makes it harder to map test results back to the BTT tree and conflicts with the PR goal of splitting compound tests. Consider splitting this into two tests (or collapsing the tree leaves into one if they must stay together).

Copilot · 2026-04-16T01:06:08Z

+    function test_WhenProposingARemoval() external {
+        // it should create removal proposal
+        // it should require fewer votes than addition
        vm.prank(RECIPIENT_1);


test_WhenProposingARemoval covers both “create removal proposal” and “require fewer votes than addition”, but the corresponding .tree file models these as separate leaves. To keep the BTT structure and test output aligned, split into two tests or adjust the tree to a single leaf.

Copilot · 2026-04-16T01:06:08Z

+    function test_WhenAProposalExpires() external {
+        // it should reject votes on expired proposals
+        // it should reject execution of expired proposals


test_WhenAProposalExpires asserts both “reject votes” and “reject execution”, but VotingRecipientRegistry.tree lists these as separate leaves. Consider splitting into two tests so failures are attributable to a single leaf, or merge the leaves in the tree.

Copilot · 2026-04-16T01:06:09Z

+    // Helper functions
+
+    function _createVoteDigest(address voter, uint256[] memory points, uint256 nonce) internal view returns (bytes32) {
+        bytes32 voteTypehash = keccak256("Vote(address voter,bytes32 pointsHash,uint256 nonce)");
+
+        bytes32 structHash = keccak256(abi.encode(voteTypehash, voter, keccak256(abi.encodePacked(points)), nonce));
+
+        return keccak256(abi.encodePacked("\x19\x01", votingModule.DOMAIN_SEPARATOR(), structHash));
+    }


This file now has two different implementations for the same EIP-712 vote digest/signature flow (createVoteSignature and _createVoteDigest). Keeping both increases maintenance cost and the risk they diverge. Consider implementing one in terms of the other (single source of truth) and using it consistently across the tests.

- Fix NatSpec grammar: 'reached' → 'has reached' - Add MAX_QUEUE_SIZE boundary tests for both addition and removal queues

Add .tree specification files for all 13 test contracts and restructure test functions to follow Branching Tree Technique naming conventions. All 115 tests pass with no logic changes — only function names and organization updated.

…k-tests # Conflicts: # test/RecipientRegistry.t.sol

exo404 added 4 commits April 15, 2026 18:51

feat: switch to mapping instead of linear search-

11c8d19

fix: add natspec to recipient mapping

26b3f54

fix: add a maximum queue size constant to gas bound functions

b9919ed

test: update tests

9bc3f28

RonTuretzky mentioned this pull request Apr 15, 2026

test: add fuzz testing suite for protocol invariants (#64) #120

Merged

3 tasks

RonTuretzky requested a review from Copilot April 16, 2026 01:01

Copilot started reviewing on behalf of RonTuretzky April 16, 2026 01:01 View session

Copilot AI reviewed Apr 16, 2026

View reviewed changes

RonTuretzky added 3 commits April 15, 2026 21:25

fix: address review feedback on queue mapping fix

baa84d5

- Fix NatSpec grammar: 'reached' → 'has reached' - Add MAX_QUEUE_SIZE boundary tests for both addition and removal queues

fix: address Copilot review feedback

8509224

RonTuretzky force-pushed the RonTuretzky/63-bulloak-tests branch from 8b992ee to 8509224 Compare April 16, 2026 01:26

RonTuretzky changed the base branch from RonTuretzky/90-queue-mapping-fix to main May 18, 2026 03:49

Merge remote-tracking branch 'origin/main' into RonTuretzky/63-bulloa…

b99e63a

…k-tests # Conflicts: # test/RecipientRegistry.t.sol

RonTuretzky merged commit 5ef7f3b into main May 18, 2026
1 of 3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: migrate all tests to Bulloak BTT format (#63)#119

refactor: migrate all tests to Bulloak BTT format (#63)#119
RonTuretzky merged 8 commits into
mainfrom
RonTuretzky/63-bulloak-tests

RonTuretzky commented Apr 15, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 16, 2026

Uh oh!

Copilot AI Apr 16, 2026

Uh oh!

Copilot AI Apr 16, 2026

Uh oh!

Copilot AI Apr 16, 2026

Uh oh!

Copilot AI Apr 16, 2026

Uh oh!

Copilot AI Apr 16, 2026

Uh oh!

Copilot AI Apr 16, 2026

Uh oh!

Copilot AI Apr 16, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	function test_WhenRemovingNon_existentRecipient() external {
	function test_WhenRemovingNonExistentRecipient() external {

	function test_RevertWhen_RemovingANon_existentRecipient() external {
	function test_RevertWhen_RemovingANonExistentRecipient() external {

Conversation

RonTuretzky commented Apr 15, 2026

Summary

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants