Skip to content

Conversation

ddoktorski
Copy link
Contributor

@ddoktorski ddoktorski commented Aug 25, 2025

@ddoktorski ddoktorski requested a review from a team as a code owner August 25, 2025 16:24
@ddoktorski ddoktorski requested review from cptartur and MKowalski8 and removed request for a team August 25, 2025 16:24
@ddoktorski ddoktorski force-pushed the spr/master/a6d72598 branch 2 times, most recently from b04b0dc to f83207c Compare August 26, 2025 11:19
@ddoktorski ddoktorski force-pushed the spr/master/012afbae branch from 6acbf3d to 3586a93 Compare August 26, 2025 11:19
@@ -189,6 +192,41 @@ pub fn deploy_at_wrapper(
Ok(contract_address)
}

fn deploy_helper(
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need this helper, doesn't this mean we are effectively testing the logic of the helper instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At this point it is kind of in between, but in #3695 it is used only as a setup for cheatnet tests

Copy link
Member

Choose a reason for hiding this comment

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

Let's make sure it's only used for setup then.

@ddoktorski ddoktorski requested a review from cptartur August 28, 2025 14:16
Towards #3659

commit-id:a6d72598
@ddoktorski ddoktorski force-pushed the spr/master/a6d72598 branch from 2bfbd44 to 9f041d9 Compare August 29, 2025 06:53
@ddoktorski ddoktorski changed the title Remove deploy cheatcode tests Remove deploy cheatnet tests Aug 29, 2025
@ddoktorski ddoktorski force-pushed the spr/master/012afbae branch from 63fb727 to 5b60eb3 Compare August 29, 2025 06:53
@@ -189,6 +192,41 @@ pub fn deploy_at_wrapper(
Ok(contract_address)
}

fn deploy_helper(
Copy link
Member

Choose a reason for hiding this comment

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

Let's make sure it's only used for setup then.

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