Skip to content

starknet_committer: move fetch_patricia_paths tests#13960

Open
ArielElp wants to merge 1 commit into
ariel/fetch_patricia_paths_testsfrom
ariel/move_tests_module
Open

starknet_committer: move fetch_patricia_paths tests#13960
ArielElp wants to merge 1 commit into
ariel/fetch_patricia_paths_testsfrom
ariel/move_tests_module

Conversation

@ArielElp
Copy link
Copy Markdown
Contributor

@ArielElp ArielElp commented May 5, 2026

No description provided.

@reviewable-StarkWare
Copy link
Copy Markdown

This change is Reviewable

Copy link
Copy Markdown
Contributor Author

ArielElp commented May 5, 2026

@cursor
Copy link
Copy Markdown

cursor Bot commented May 7, 2026

PR Summary

Low Risk
Low risk: changes are limited to test module wiring and fixture include paths, with no production logic modifications.

Overview
Moves the fetch_patricia_paths_inner test module to live alongside trie_traversal.rs and updates the #[cfg(test)] #[path = ...] include accordingly.

Fixes the include_str! paths for the Python-generated JSON fixtures so the moved tests continue to load their resources correctly.

Reviewed by Cursor Bugbot for commit 3f8b451. Bugbot is set up for automated code reviews on this repo. Configure here.

Copy link
Copy Markdown
Contributor

@yoavGrs yoavGrs left a comment

Choose a reason for hiding this comment

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

@yoavGrs partially reviewed 2 files and made 2 comments.
Reviewable status: 2 of 3 files reviewed, 2 unresolved discussions (waiting on ArielElp).


crates/starknet_committer/src/db.rs line 13 at r1 (raw file):

#[cfg(test)]
mod fetch_patricia_paths_tests;

Define it in trie_traversal.rs.

Code quote:

#[cfg(test)]
mod fetch_patricia_paths_tests;

crates/starknet_committer/src/db/trie_traversal.rs line 57 at r1 (raw file):

#[cfg(test)]
#[path = "facts_db/traversal_test.rs"]

According to code quality, it should stay here.

Suggestion:

#[path = "traversal_test.rs"]

@ArielElp ArielElp force-pushed the ariel/move_tests_module branch from 621d40d to 6d471bd Compare May 14, 2026 08:43
@ArielElp ArielElp force-pushed the ariel/fetch_patricia_paths_tests branch from 22604c1 to 4c3049b Compare May 14, 2026 08:43
@ArielElp ArielElp force-pushed the ariel/move_tests_module branch from 6d471bd to 3f8b451 Compare May 14, 2026 09:03
Copy link
Copy Markdown
Contributor Author

@ArielElp ArielElp left a comment

Choose a reason for hiding this comment

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

@ArielElp partially reviewed 2 files and all commit messages, and made 2 comments.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on yoavGrs).


crates/starknet_committer/src/db.rs line 13 at r1 (raw file):

Previously, yoavGrs wrote…

Define it in trie_traversal.rs.

Done.


crates/starknet_committer/src/db/trie_traversal.rs line 57 at r1 (raw file):

Previously, yoavGrs wrote…

According to code quality, it should stay here.

You mean under facts_db, it's weird now that it's generic right?
In any case, I moved the definition to trie_traversal.

Copy link
Copy Markdown
Contributor

@yoavGrs yoavGrs left a comment

Choose a reason for hiding this comment

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

@yoavGrs reviewed 2 files and all commit messages, and resolved 2 discussions.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on ArielElp).

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.

3 participants