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

Add a couple of tools to help with debugging integration tests #1597

Merged
merged 2 commits into from
May 15, 2023

Conversation

ilyagr
Copy link
Collaborator

@ilyagr ilyagr commented May 11, 2023

These are a couple of tools I made to help with debugging #864. I was only partially successful, but I think they can be useful regardless.

@ilyagr ilyagr changed the title Add a couple of tools to help debugging Add a couple of tools to help with debugging integration tests May 11, 2023
Copy link
Owner

@martinvonz martinvonz left a comment

Choose a reason for hiding this comment

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

Before this, it was difficult to run an integration test after adding any directives
from printf-style debugging to jj (e.g. err!, eprintln!, println!), since
jj_cmd_success fails if jj to output anything to stderr while jj_cmd_failure
fails if stdout is not empty.

FYI, there's also jj_cmd (without _success or _failure) which doesn't check anything

@martinvonz
Copy link
Owner

Before this, it was difficult to run an integration test after adding any directives
from printf-style debugging to jj (e.g. err!, eprintln!, println!), since
jj_cmd_success fails if jj to output anything to stderr while jj_cmd_failure
fails if stdout is not empty.

FYI, there's also jj_cmd (without _success or _failure) which doesn't check anything

I now realize that you didn't want to test the output. Sorry :)

@ilyagr
Copy link
Collaborator Author

ilyagr commented May 11, 2023

FYI, there's also jj_cmd (without _success or _failure) which doesn't check anything

Yes, before this PR, the way to do printf-style debugging would be to change each jj_cmd_success to jj_cmd and then extract stdout/stderr from the result. I did it once; it's not fun.

tests/common/mod.rs Outdated Show resolved Hide resolved
@ilyagr
Copy link
Collaborator Author

ilyagr commented May 11, 2023

As a complement to this, I'm thinking of creating a TestEnvironment::preserve_repo_for_each_jj_invocation that preserves the repo state before each jj_cmd command in a separate directory. This would allow debugging with gdb/lldb at that exact repo state.

(Alternatively, we could only preserve the last state and rely on the operation log, but that would be annoying when testing behavior that does not exist for jj --at-op)

I'm not 100% sure that would be a worthwhile endeavor; perhaps I will instead just get used to recreating the desired repo state myself.

@ilyagr ilyagr force-pushed the dbg-tools branch 3 times, most recently from 95fb261 to 4f3e67f Compare May 15, 2023 02:40
@ilyagr ilyagr enabled auto-merge (rebase) May 15, 2023 02:43
@ilyagr ilyagr disabled auto-merge May 15, 2023 02:46
…f debugging

Before this, it was difficult to run an integration test after adding any
directives from printf-style debugging to jj (e.g. `err!`, `eprintln!`,
`println!`), since `jj_cmd_success` fails if `jj` to output anything to stderr
while `jj_cmd_failure` fails if stdout is not empty.

This adds a `TestEnvironment::debug_allow_stderr` variable that lifts this
restriction for `jj_cmd_success` and makes it output anything `jj` output to
stderr instead. You can set it directly or by running the test with the
`DEBUG_ALLOW_STDERR` environment variable set. You can then add `err!`
anywhere.

You do need to run the test in a somewhat special way, as described in the
docstring.
@ilyagr ilyagr enabled auto-merge (rebase) May 15, 2023 02:49
@ilyagr ilyagr merged commit 7c24e7b into martinvonz:main May 15, 2023
@ilyagr ilyagr deleted the dbg-tools branch May 15, 2023 03:02
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.

2 participants