-
Notifications
You must be signed in to change notification settings - Fork 7
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
fix: disable prettydiff output which will seg fault #8
Conversation
Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
In which env, are you having this issue? |
both intel and apple chip macOS can reproduce this bug, with rust toolchain |
Interesting, it seems this bug came from nightly, I can't reproduce using stable. |
Yes, I switch my toolchain from nightly-2022-07 to nightly-2022-12 and find this problem. Haven't dug why. |
@@ -237,8 +237,16 @@ impl<E: Environment> Runner<E> { | |||
.names("Expected", "Actual"); | |||
let is_different = diff.diff().iter().any(|d| !matches!(d, DiffOp::Equal(_))); | |||
if is_different { | |||
println!("Result unexpected, path:{:?}\n", path.as_ref()); | |||
diff.prettytable(); | |||
println!("Result unexpected, path:{:?}", path.as_ref()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually we can just print diff
println!(
"Result unexpected, path:{:?}, diff:\n{}\n",
path.as_ref(),
diff
);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, only prettytable
will segfault. But I also think it's a bit spam when you make a general change that might affect many cases. This function will display lots of not-difference, which I think is not very friendly.
We can make this an option afterward, whether or not the table format got fixed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Signed-off-by: Ruihang Xia waynestxia@gmail.com
Which issue does this PR close?
Closes #
Rationale for this change
the
prettytable
will lead to a seg fault in my envWhat changes are included in this PR?
Haven't found out the reason. Disable it for now, display a diff command hint instead.
Are there any user-facing changes?
How does this change test