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

TextDiffReporter should not throw if files differ #106

Closed
claremacrae opened this issue Feb 22, 2020 · 3 comments
Closed

TextDiffReporter should not throw if files differ #106

claremacrae opened this issue Feb 22, 2020 · 3 comments
Assignees

Comments

@claremacrae
Copy link
Collaborator

This needs checking, but I believe that using TextDiffReporter will change the behaviour of failing tests - from throwing the Approvals-specific mismatch exception - to throwing an exception to indicate that the “system call failed”.

This is because diff returns a non-zero exit code when files differ.

We already saw this in our own tests, but didn’t appreciate the consequences at the time.

@claremacrae
Copy link
Collaborator Author

Confirmed - this is a real problem.

If I have a failing test due to mismatch of approved and received, the expected console output is like this:

.../MyTest.cpp:8: ERROR: test case THREW exception: Failed Approval: 
Received does not match approved 
Received : "out_of_source_test.out_of_source_sample.received.txt" 
Approved : "out_of_source_test.out_of_source_sample.approved.txt"

If the test is run with TextDiffReporter, the output is like this:

APPROVAL_TESTS_USE_REPORTER=TextDiffReporter ./out_of_source
.../MyTest.cpp:8: ERROR: test case THREW exception: "diff" "out_of_source_test.out_of_source_sample.received.txt" "out_of_source_test.out_of_source_sample.approved.txt": failed with exit code 256

@claremacrae
Copy link
Collaborator Author

This matters because in CI builds, the default reporter is TextDiffReporter, so it makes Approval Tests failures in CI builds confusing.

claremacrae added a commit that referenced this issue Apr 20, 2021
Co-Authored-By: Llewellyn Falco <isidore@users.noreply.github.com>
@claremacrae claremacrae self-assigned this Apr 21, 2021
claremacrae added a commit that referenced this issue Apr 21, 2021
This should fix the DocTest_Tests on Windows
claremacrae added a commit that referenced this issue Apr 21, 2021
@claremacrae
Copy link
Collaborator Author

This has been fixed - closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant