-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
Description
This is a follow-up to #36879, to request we reconsider the effect this change had on how tests which pass are displayed.
That PR changed Tests.Pass objects to store extra info about the expression and data passed to @test, but storing this info also led to a change in what was displayed when a test passes. Before only "Test Passed" was displayed, and after the "Expression/Evaluated" was also printed e.g.
In Julia v1.6:
julia> @test 1 == 1
Test PassedIn Julia v1.7:
julia> @test 1 == 1
Test Passed
Expression: 1 == 1
Evaluated: 1 == 1The PR was intending to address issue #25483 which only requested the info be stored, i believe so that custom Testsets could make use of this info (e.g. display it), not also suggesting a change to what's displayed by Test.@test. So it seems that the change to the printed output was not particularly requested by anyone. And I think the change is a little unfortunate for a couple reasons:
- It is a significant interface change for those who run lots of tests in the REPL. Previously, verbose output (showing Expression/Evaluated) indicated that the test had failed. Seeing this output has come to indicate "something's wrong".
- The printing of the Expression/Evaluated info can easily push the most crucial information off the screen i.e. did the test pass or not? This was not so much of an issue before when verbose output itself indicated the test did not pass.
As an example, here's how a passing tests comparing two DataFrames looks in the REPL:
julia> X = rand(300, 300);
julia> df = DataFrame(X, :auto);
julia> @test df == df@mmiller-max points out (on Slack) that the printing change resulting from the PR were only briefly discussed (see this discussion). And that the conclusion was that changes to the printing code (which were originally in the PR to keep printing behaviour closer to what it was before the change) weren't really applicable to the PR and should be removed from the PR. So leaves it open for a further discussion, I think, what the actual printing behaviour should be.
Personally I think the behaviour should be that tests which pass display only "Test Passed".
@mmiller-max says one reason for not implementing printing code changes in that PR was that it was actually inconsistent as to what a passing test displayed on v1.6, as @test_throws still showed some additional output, unlike standard @test:
julia> @test_throws ArgumentError throw(ArgumentError("a"))
Test Passed
Thrown: ArgumentErrorIf we wanted to remove this inconsistency, we could only show "Test Passed" for @test_throws too (although i'm not so sure this change is needed in the name of consistency, since it doesn't have the same issue of being an interface change or prone to verbose output). Either way, I suggest we change @test back to only showing "Test Passed", since the verbose output actually masks the crucial info of whether or not the test passed.

