Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(cheatcodes): forge execution context check #7377

Merged
merged 15 commits into from
Apr 9, 2024

Conversation

grandizzy
Copy link
Collaborator

@grandizzy grandizzy commented Mar 12, 2024

Motivation

Closes #2900

Solution

Add a global /execution context based on forge subcommand used - can be set only once per program, and it can be checked by using isContext(ForgeContext context) env cheatcode.
ForgeContext have following options:

  • TestGroup: Test group execution context (test, coverage or snapshot).
  • Test
  • Coverage
  • Snapshot
  • ScriptGroup: Script group execution context (dry run, broadcast or resume)
  • ScriptDryRun
  • ScriptBroadcast
  • ScriptResume
  • Unknown

@grandizzy grandizzy marked this pull request as ready for review March 13, 2024 08:56
@grandizzy grandizzy changed the title [WIP] feat(cheatcodes): forge execution context check feat(cheatcodes): forge execution context check Mar 13, 2024
@grandizzy grandizzy requested a review from Evalir as a code owner April 1, 2024 06:45
crates/cheatcodes/src/env.rs Outdated Show resolved Hide resolved
crates/cheatcodes/src/env.rs Outdated Show resolved Hide resolved
@grandizzy grandizzy requested a review from DaniPopes April 5, 2024 13:43
Copy link
Member

@DaniPopes DaniPopes left a comment

Choose a reason for hiding this comment

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

lgtm, pending another review. CC @mds1 @mattsse @klkvr

crates/forge/tests/cli/context.rs Outdated Show resolved Hide resolved
Copy link
Member

@Evalir Evalir left a comment

Choose a reason for hiding this comment

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

lgtm! test failures seem spurious, so waiting on another review

@grandizzy
Copy link
Collaborator Author

lgtm! test failures seem spurious, so waiting on another review

yeah, according to https://github.com/foundry-rs/foundry/blob/master/docs/dev/cheatcodes.md

The initial execution of this command, following the addition of a new cheat code, will result in an
update to the JSON files, which is expected to fail. This failure is necessary for the CI system to
detect that changes have occurred. Subsequent executions should pass, confirming the successful
update of the files.```

@DaniPopes
Copy link
Member

It's not spurious, you have to run the command locally as the output and doc suggest

Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

lgtm

@mattsse
Copy link
Member

mattsse commented Apr 9, 2024

@grandizzy I ran cargo cheats, which perhaps broke a few things in sol files

@grandizzy
Copy link
Collaborator Author

@grandizzy I ran cargo cheats, which perhaps broke a few things in sol files

@mattsse yeah, sorry, my bad, was something I missed for adding the new enum, should be fixed now (forced pushed your commit). @Evalir 's comment re CI failing was a legit concern 🤦‍♂️

@Evalir
Copy link
Member

Evalir commented Apr 9, 2024

yep @grandizzy sorry for the confusion here—first CI failure comment seemed spurious but below there was the cargo cheats message

@Evalir Evalir merged commit a6d6a3a into foundry-rs:master Apr 9, 2024
19 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.

Ability to detect different "environments" in scripts/tests
6 participants