Skip to content

Fix pretty printing of symbols by Test.jl#57430

Closed
nerdberg792 wants to merge 8 commits intoJuliaLang:masterfrom
nerdberg792:pretty
Closed

Fix pretty printing of symbols by Test.jl#57430
nerdberg792 wants to merge 8 commits intoJuliaLang:masterfrom
nerdberg792:pretty

Conversation

@nerdberg792
Copy link

@nerdberg792 nerdberg792 commented Feb 16, 2025

Fixes #57287
When we perform a Test :

julia> x = 1; y = :sym; @test x == y
Test Failed at REPL[48]:1
  Expression: x == y
   Evaluated: 1 == sym

But ideally the test should print Evaluated 1 == :sym

I have changed the parsing of the arguments :

Updated Test.jl and added some if else conditions :

now the tests mentioned in the issue yields these results :

julia> x = 1; y = :sym; @test x == y
Test Failed at REPL[2]:1
  Expression: x == y
   Evaluated: 1 == :sym

julia> x = 1; y = :(1+1); @test x == y
Test Failed at REPL[2]:1
  Expression: x == y
   Evaluated: 1 == :(1 + 1)

@DilumAluthge
Copy link
Member

Can you edit the PR title to be a bit more descriptive, e.g. to briefly describe the bug that is being fixed.

Removed whitespaces
@nerdberg792
Copy link
Author

Can you edit the PR title to be a bit more descriptive, e.g. to briefly describe the bug that is being fixed.

I have made some changes can you review my PR , also I haven't added any tests to check for the aforementioned bug

removed whitespaces
removed whitespace from line 367
removeed whitespaces
@KristofferC KristofferC added the needs tests Unit tests are required for this change label Feb 16, 2025
@nerdberg792
Copy link
Author

@KristofferC can you please point out where should I add the tests

@LilithHafner
Copy link
Member

stdlib/Test/test/runtests.jl

@vchuravy vchuravy changed the title updated Test.jl (Solves #57287) Fix pretty printing of symbols by Test.jl Feb 17, 2025
@KristofferC
Copy link
Member

The last example in the first post is is already what is shown on master so not sure what is meant by that.

quoted_args[i+2] = b
if a isa Symbol
quoted_args[i] = QuoteNode(a)
elseif Meta.isexpr(a, :call)
Copy link
Member

Choose a reason for hiding this comment

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

In which case is this used? What difference does it make?

quoted_args[i] = a
quoted_args[i+2] = b
if a isa Symbol
quoted_args[i] = QuoteNode(a)
Copy link
Member

Choose a reason for hiding this comment

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

I think it woud be better to put the whole logic into a function so it doesn't have to be repeated for both a and b.

Copy link
Member

Choose a reason for hiding this comment

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

That function is the existing Base.quoted function in this case

On the previous changes , I misinterpret how the expressions need to be printed , fixed this here
@nerdberg792
Copy link
Author

The last example in the first post is is already what is shown on master so not sure what is meant by that.

I misinterpreted your comments on the issues , I have now made the necessary changes and it seems to work fine , can you please review this once , sorry for the inconvenience cause !

@LilithHafner
Copy link
Member

This looks good so far. Would you please address Jameson's comment by using Base.quoted and Kristoffer's comment by adding tests that fail on master and pass on this PR?

@DilumAluthge
Copy link
Member

@nerdberg792 Are you still interested in working on this?

@vtjnash
Copy link
Member

vtjnash commented Feb 24, 2026

Closed by #61054

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

Labels

needs tests Unit tests are required for this change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Failure to quote symbols in pretty printing in @test failures

5 participants