Skip to content
This repository was archived by the owner on Apr 16, 2026. It is now read-only.

plumb enable vm tracing - #145

Merged
cali-jumptrading merged 1 commit into
mainfrom
cali/plumb-enable-vm-tracing
Aug 29, 2025
Merged

plumb enable vm tracing#145
cali-jumptrading merged 1 commit into
mainfrom
cali/plumb-enable-vm-tracing

Conversation

@cali-jumptrading

Copy link
Copy Markdown
Contributor

No description provided.

@cali-jumptrading
cali-jumptrading force-pushed the cali/plumb-enable-vm-tracing branch 6 times, most recently from 2055281 to a54ff1b Compare August 29, 2025 17:40
@mjain-jump
mjain-jump self-requested a review August 29, 2025 17:46
Comment thread src/test_suite/test_suite.py Outdated
shutil.rmtree(globals.output_dir)
globals.output_dir.mkdir(parents=True, exist_ok=True)

os.environ.pop("ENABLE_VM_TRACING", None)

Copy link
Copy Markdown
Collaborator

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 here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

cuz it's an environment variable so we need to make sure it's fully cleared when we don't want to enable vm tracing

Comment thread src/test_suite/test_suite.py Outdated
@cali-jumptrading
cali-jumptrading force-pushed the cali/plumb-enable-vm-tracing branch from a54ff1b to fc36a53 Compare August 29, 2025 18:15
Comment thread src/test_suite/test_suite.py Outdated
Comment on lines +415 to +419
enable_vm_tracing: bool = typer.Option(
False,
"--enable-vm-tracing",
"-evm",
help="Enable FD VM tracing",
),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

vm tracing probably isnt needed for run_tests, stdout is not captured properly anyways because tests may be run in parallel so the output wont be legible

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I added the option to run-tests because I sometimes use run-tests to run both agave and firedancer for a single target.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

when tracing is added to agave its going to print garbage into stdout

Comment thread src/test_suite/test_suite.py Outdated
@cali-jumptrading
cali-jumptrading force-pushed the cali/plumb-enable-vm-tracing branch from fc36a53 to 8375d9e Compare August 29, 2025 18:24
@cali-jumptrading
cali-jumptrading force-pushed the cali/plumb-enable-vm-tracing branch from 8375d9e to f4079b0 Compare August 29, 2025 18:28

@mjain-jump mjain-jump left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

thank you!

@cali-jumptrading
cali-jumptrading merged commit 43ad081 into main Aug 29, 2025
4 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants