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

Change test log messages to be more readable #8940

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Hex052
Copy link

@Hex052 Hex052 commented May 7, 2023

This is more to get feedback than anything, because I haven't checked to for tests that would break (I don't believe there are any).

Current problem

If you have a detailed error message that gets written, it calls show on the string and removes any formatting that might have been applied. For example, I have something that tests if two items are equal and sets the message to "Expected: " ++ show expected ++ "\nActual : " ++ show actual.

Proposed solution

It would be more useful if the test logs displayed the message directly. read is never used on the log files, so being able to do the inverse of show isn't very useful there.

Potential changes

Rather than what I have proposed, perhaps display the message on the next line, and indent it (and all lines of the message) to be more visually obvious.
Additionally, there appears to be a line length limit in the logs, which makes it weird to be able to read any messages created (if it wraps them at a weird spot, or something). Maybe this should be configurable or able to be disabled entirely?


Please include the following checklist in your PR:

Bonus points for added automated tests!

@ulysses4ever
Copy link
Collaborator

Hey! Thanks for trying this out! Do you know about cabal test --test-show-details=direct? There's a ticket to make it the default (#7817). Generally, we want to rely on testing frameworks (hspec, tasty, sidtest) to print messages like "Fail" or "Pass". In combination with direct style (see above) that's usually good enough.

@Kleidukos
Copy link
Member

Hi @Hex052 and thank you for taking this initiative. :)
I'd like to echo the words of @ulysses4ever and suggest that #7817 be implemented. :)

Of course do not hesitate to ask if you have any kind of question or blocker, we'd be delighted to help the best we can.

@ulysses4ever
Copy link
Collaborator

@Kleidukos i even have a local branch that flips the default. I never got to publishing it because I failed to build the docs! (Some Python nonsense. Since then I dream about a nix-shell for the docs and nothing else...) What makes the task tricker: docs for cabal test are practically non-existent. Whoever flips the switch, would better add some docs...

@ulysses4ever
Copy link
Collaborator

@Kleidukos please, review: #8942

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.

None yet

3 participants