Skip to content

Conversation

@Idanh-starkware
Copy link
Contributor

@Idanh-starkware Idanh-starkware commented Aug 20, 2025

Type

  • feature
  • bugfix
  • dev (no functional changes, no API changes)
  • fmt (formatting, renaming)
  • build
  • docs
  • testing

Description

Breaking changes?

  • yes
  • no

This change is Reviewable

Copy link
Contributor Author

Idanh-starkware commented Aug 20, 2025

This stack of pull requests is managed by Graphite. Learn more about stacking.

@Idanh-starkware Idanh-starkware marked this pull request as ready for review August 20, 2025 08:03
@Idanh-starkware Idanh-starkware force-pushed the idanh/adding_builtin_usage_tests branch from 2d123be to 932e76d Compare August 20, 2025 08:05
@Idanh-starkware Idanh-starkware force-pushed the idanh/enableing_execute_task_tests branch from e577a4e to e061dd6 Compare August 20, 2025 08:05
@Idanh-starkware Idanh-starkware self-assigned this Aug 20, 2025
@Idanh-starkware Idanh-starkware force-pushed the idanh/adding_builtin_usage_tests branch from 932e76d to fa20c55 Compare August 20, 2025 12:36
@Idanh-starkware Idanh-starkware force-pushed the idanh/enableing_execute_task_tests branch 2 times, most recently from 4b91e6d to 722e685 Compare August 20, 2025 12:50
@Idanh-starkware Idanh-starkware force-pushed the idanh/adding_builtin_usage_tests branch 2 times, most recently from 2a4f194 to 9f4b9f2 Compare August 20, 2025 12:51
Copy link
Contributor

@OmriEshhar1 OmriEshhar1 left a comment

Choose a reason for hiding this comment

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

@OmriEshhar1 reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @YairVaknin-starkware)


crates/cairo-program-runner-lib/src/hints/builtin_usage_hints.rs line 284 at r1 (raw file):

    #[rstest(finalize, case(true), case(false))]
    fn test_builtin_usage_add_other_segment(finalize: bool) {
        // Test the builtin_usage_add_other_segment function with finalize set to true.

it is tested for both finalize=true and finalize=false, isn't it?

Code quote:

with finalize set to true

@Idanh-starkware Idanh-starkware force-pushed the idanh/adding_builtin_usage_tests branch from 9f4b9f2 to db67973 Compare August 21, 2025 14:38
@Idanh-starkware Idanh-starkware force-pushed the idanh/enableing_execute_task_tests branch from 722e685 to 1a731f2 Compare August 21, 2025 14:38
@Idanh-starkware
Copy link
Contributor Author

crates/cairo-program-runner-lib/src/hints/builtin_usage_hints.rs line 284 at r1 (raw file):

Previously, OmriEshhar1 wrote…

it is tested for both finalize=true and finalize=false, isn't it?

Yes, bad documentation.

@Idanh-starkware Idanh-starkware force-pushed the idanh/adding_builtin_usage_tests branch from db67973 to c022280 Compare August 21, 2025 14:43
Copy link
Contributor

@OmriEshhar1 OmriEshhar1 left a comment

Choose a reason for hiding this comment

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

@OmriEshhar1 reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @YairVaknin-starkware)

@Idanh-starkware Idanh-starkware merged commit 9dc011e into idanh/enableing_execute_task_tests Aug 25, 2025
6 checks passed
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