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

Make --test-show-details=direct the default #7817

Closed
konsumlamm opened this issue Nov 14, 2021 · 22 comments · Fixed by #8942
Closed

Make --test-show-details=direct the default #7817

konsumlamm opened this issue Nov 14, 2021 · 22 comments · Fixed by #8942

Comments

@konsumlamm
Copy link

konsumlamm commented Nov 14, 2021

Description
Currently, the default is failures, which is not a good default (more details below). IMO, the default should be direct instead, which does the things a new user (or at least I) would expect.

Comparison of the filters

  • always: only shows the results after the test suite has finished (this behavior is quite unintuitive and it had lead me and others to believe it was a bug, see cabal test suppressing output #1581)
  • never: doesn't show any results (this is obviously a bad default)
  • failures: only shows failing test suites and only after they finished (this may be occasionally useful, but isn't a good default imo)
  • streaming: shows the results in real time, but doesn't support colors
  • direct: shows the results in real time and supports colors (in fact, it is the only filter supporting colors)

So in summary, I think direct is the best default option (and it also matches the default behavior of stack test).

@fendor
Copy link
Collaborator

fendor commented Nov 14, 2021

I agree, this should be the default. Personally, I usually opt for cabal run <name-of-testsuite> since it does already do roughly that (and it is way easier to pass arguments to the testsuite).

@fgaz
Copy link
Member

fgaz commented Nov 14, 2021

Currently, the default is always

is it? on my machine (linux, cabal 3.7) it's "failures", and I think that's reasonable, since successful tests don't really carry any extra information

@konsumlamm
Copy link
Author

I agree, this should be the default. Personally, I usually opt for cabal run <name-of-testsuite> since it does already do roughly that (and it is way easier to pass arguments to the testsuite).

That is a good alternatve (I also sometimes use it for passing arguments). I wonder why cabal test isn't just an alias for cabal run <name of test-suite> in the first place.

Currently, the default is always

is it? on my machine (linux, cabal 3.7) it's "failures", and I think that's reasonable, since successful tests don't really carry any extra information

Oh, you're right, but that doesn't change my points. It still has the same problem that results are only shown after the test suite finished. As for successful tests not carrying any extra information, I think it's satisfying to see the passing tests and also useful if you wanna know which tests exactly are run (cargo test also does that by default fwiw).

@phadej
Copy link
Collaborator

phadej commented Nov 14, 2021

That is a good alternatve (I also sometimes use it for passing arguments). I wonder why cabal test isn't just an alias for cabal run in the first place.

Multiple test suites, cwd changes, ...

@fendor
Copy link
Collaborator

fendor commented Nov 23, 2021

successful tests don't really carry any extra information

I disagree, seeing that tests are run is very valuable feedback: You have a way easier time telling whether your tests are stuck, what progress is made, etc... I have yet to encounter a single use-case, where I was glad about the default.

In fact, I argue exactly the opposite: Everyone cares about the tests output, unless you don't (e.g. in CI successful tests can be probably ignored.)

@gbaz
Copy link
Collaborator

gbaz commented Nov 23, 2021

Right -- I think there are two use cases. In the CI case we really don't want to clutter already potentially super noisy logs with anything and only want failures. (This is generally the case with any sort of automation imho). In the interactive "fixing tests" case we want the nicest most interactive output we can have. I'm ok with switching the default to direct as long as we signal this enough so that everyone can clean up their CI. That said, I'm also curious what the use case is for streaming at all given that direct seems to be basically superior?

@phadej
Copy link
Collaborator

phadej commented Nov 23, 2021

directisn't superb:

EDIT: haskell-ci uses --test-show-details=direct by default, but has an option to turn it off. It's annoying when it doesn't work though.

@gbaz
Copy link
Collaborator

gbaz commented Nov 23, 2021

I don't understand how the linked issue is coupled to direct? It does sound like the custom build type issue is an obstacle to this request, unless we either automatically downgrade direct to streaming, or add direct support to old cabal.

@phadej
Copy link
Collaborator

phadej commented Nov 24, 2021

Err. I confused always with direct.

@Mikolaj
Copy link
Member

Mikolaj commented Dec 28, 2021

I don't understand how the linked issue is coupled to direct? It does sound like the custom build type issue is an obstacle to this request, unless we either automatically downgrade direct to streaming, or add direct support to old cabal.

Err. I confused always with direct.

Does this mean we are unblocked? It would be great to move forward with this ticket.

@phadej
Copy link
Collaborator

phadej commented Dec 28, 2021

Does this mean we are unblocked? It would be great to move forward with this ticket.

The

Also old Cabal (used when build-type: Custom) doesn't have direct option. That broke on some of Kmett's packages when they still used build-type: Custom.

part is still there.

EDIT: please nuke build-type: Custom. Your life will be easier. ;)

@KristianBalaj
Copy link

KristianBalaj commented Jul 6, 2022

direct option does not log anything helpful into the test log file which is bad.
EDIT: it should log at least failures, it's easier to go over the failures in a log file than to scroll in a console without the ability to collapse.

@Mikolaj
Copy link
Member

Mikolaj commented Jul 6, 2022

That's a good remark. Should we enable logging in direct or do we need yet another option? How resource-intensive the logging could be? Probably not much?

@LightAndLight
Copy link

LightAndLight commented Jul 12, 2022

The --test-show-details=direct flag makes cabal test my-library behave the way I expect. I normally use cabal run my-library-tests, but it doesn't always work.

@KristianBalaj
Copy link

@Mikolaj I would love to see the behaviour of direct to be the way it is in the console and additionally I would love to see the logging into a file to be the way it is with the always case.
After this upgrade the direct option would be a great default 👌

@Mikolaj
Copy link
Member

Mikolaj commented Jul 12, 2022

We could complicate and add yet another option that controls logging, but if nobody objects, let's simply do what you propose. If there are existing or future use cases where the logs are huge and undesired, we can implement the no-logs flag separately in the future. A pull request would be very welcome (unless somebody objects within a few days).

@Mikolaj
Copy link
Member

Mikolaj commented Jul 12, 2022

And then I think we can make direct the default (for cabal 3.10).

@ulysses4ever
Copy link
Collaborator

ulysses4ever commented Nov 23, 2022

We just got another request to get this #8604, so I add the 3.10 milestone.

I understand the outcome of this discussion as follows:

  • we simply flip the default to direct: the behavior overall is nicer but we lose the logs
  • if people want logs, they will need to flip the switch to something else manually: this may break someone's workflow at this point, but the gain seems bigger than the loss
  • in the future, we add logging to direct (the default at that point) and add a flag to opt out of logging.

The reason the second and third are separate is because I don't expect the third step to be entirely trivial: direct has all the niceties exactly because it doesn't go through a separate file.

Or is losing logs in the default a dealbreaker?

@Mikolaj
Copy link
Member

Mikolaj commented Nov 23, 2022

Edit: this is a non-issue, see next comment.

I wonder, perhaps haskell-ci maintainers (cc @phadej, @RyanGlScott) could call cabal test with a flag that ensures logging even in 3.10 and so eliminate a lot of the potential breakage?

@ulysses4ever
Copy link
Collaborator

@Mikolaj

The comment above (#7817 (comment)) indicates that haskell-ci already uses direct by default but allow to opt-out. So we're good on this front. (Why would you need a log file in CI anyway? it's too much hussle to access it, and it's much easier to use web interface to inspect console output, which you can also download as a text file).


Reading on on the history of direct #2911, I become more certain that have both — colorful output and a log file — is not trivial: we pass a pipe to a test and it's either a terminal pipe, in which case we get colors, or a file pipe, in which case we don't. So I suggest we just flip the default switch to direct-as-it-is-today and be loud about it.

@Mikolaj
Copy link
Member

Mikolaj commented Nov 28, 2022

If haskell-CI defaults to direct, I'm even more convinced we should change the default.

@ulysses4ever
Copy link
Collaborator

Just FTR, here’s a recently generated Haskell CI script that does have direct in it:

https://github.com/haskell-hvr/HsYAML/blob/2c04f83dcf50b86bdc1fa1531acf294343b4d1f5/.github/workflows/haskell-ci.yml#L251

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

Successfully merging a pull request may close this issue.

9 participants