Skip to content

starknet_os_flow_tests: extract test contract constructor calldata helpers#14099

Open
AvivYossef-starkware wants to merge 1 commit into
05-10-starknet_os_flow_tests_test_tx_per_proof_version_and_program_hashfrom
aviv/starknet_os_flow_tests-constructor-calldata-helpers
Open

starknet_os_flow_tests: extract test contract constructor calldata helpers#14099
AvivYossef-starkware wants to merge 1 commit into
05-10-starknet_os_flow_tests_test_tx_per_proof_version_and_program_hashfrom
aviv/starknet_os_flow_tests-constructor-calldata-helpers

Conversation

@AvivYossef-starkware
Copy link
Copy Markdown
Contributor

Summary

  • Adds two helpers near the top of crates/starknet_os_flow_tests/src/tests.rs:
    • test_contract_constructor_calldata(my_storage_var_addend_1, my_storage_var_addend_2) — named constructor builder. Doc comment notes that TestContract::constructor writes arg1 + arg2 to my_storage_var.
    • default_test_contract_constructor_calldata() — calls the above with (Felt::ZERO, Felt::ZERO).
  • Replaces 19 occurrences of calldata![Felt::ZERO, Felt::ZERO] (paired with TestContract(...) everywhere) with default_test_contract_constructor_calldata().
  • Replaces 5 occurrences of calldata![Felt::ONE, Felt::TWO] with test_contract_constructor_calldata(Felt::ONE, Felt::TWO).

Addresses the review comment on #14042 (reviewable thread) — calldata![Felt::ZERO, Felt::ZERO] was opaque at the call site; the named helpers make the role of the felts (constructor args summed into my_storage_var) obvious without digging into the Cairo source.

Stacked on top of #14042 — only this one commit is new.

Test plan

  • cargo check -p starknet_os_flow_tests --tests
  • scripts/rust_fmt.sh
  • Behavior unchanged — purely mechanical rename of literals + two new helper definitions. No new logic.

@reviewable-StarkWare
Copy link
Copy Markdown

This change is Reviewable

@cursor
Copy link
Copy Markdown

cursor Bot commented May 19, 2026

PR Summary

Low Risk
Low risk: test-only refactor that replaces repeated constructor calldata literals with small helper functions, with no behavioral changes intended.

Overview
Introduces two small helpers in starknet_os_flow_tests to build TestContract constructor calldata (including a default that keeps my_storage_var at zero), documenting the constructor’s side effect.

Replaces many inline calldata![Felt::ZERO, Felt::ZERO] / calldata![Felt::ONE, Felt::TWO] occurrences across tests with these helpers to make constructor intent explicit and reduce duplication.

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

Copy link
Copy Markdown
Collaborator

@Yoni-Starkware Yoni-Starkware left a comment

Choose a reason for hiding this comment

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

:lgtm:

@Yoni-Starkware reviewed 1 file and all commit messages, and made 2 comments.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on AvivYossef-starkware).


crates/starknet_os_flow_tests/src/tests.rs line 285 at r1 (raw file):

    let (mut test_builder, [test_contract_address]) = TestBuilder::create_standard_with_config(
        [(test_contract, test_contract_constructor_calldata(Felt::ONE, Felt::TWO))],

Since it's either 0,0 or 1,2, I'd add another function for the 1,2 case. nontrivial_constructor_calldata

Code quote:

       [(test_contract, test_contract_constructor_calldata(Felt::ONE, Felt::TWO))],

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