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

An option to disable "TEST START" println? #158

Closed
aldanor opened this issue Jul 16, 2022 · 9 comments
Closed

An option to disable "TEST START" println? #158

aldanor opened this issue Jul 16, 2022 · 9 comments

Comments

@aldanor
Copy link

aldanor commented Jul 16, 2022

println!("{:-^40}", " TEST START ");

When running in IDEs (like CLion) that automatically capture output in the test runner, there's tons of --- TEST START --- printlns. It's not mission-critical, but very annoying 😢

Looking at the code, wonder why are they needed at all? I found one place where this is checked, in rstest's tests:

.take_while(|l| !l.contains("TEST START"))

But then again, I guess there's other ways to check this, without having to dump this to stdout on every test? (Or as another option, if it's really required, could make it a non-default crate feature; or yet another proc-macro attr, although that's an overkill)

If these printlns could be removed, conditionally or unconditionally, would be much appreciated.

@la10736
Copy link
Owner

la10736 commented Jul 17, 2022 via email

@aldanor
Copy link
Author

aldanor commented Jul 17, 2022

@la10736 Thanks for replying so fast!

Change it today means introduce a breaking change.

Technically - yes, agreed (however, you're not 1.0 yet, so breaking changes are kind of expected?). However, to be honest I can't picture a real situation where this change would be actually breaking anything - except for a single test in rstest's test suite itself.

Remove it when there isn't any trace attribute

That sounds like the best way to do it, I think. If you're tracing things - you're going to be looking at stdout capture anyway. If you're not tracing anything - there's no reason to write anything to stdout.

Use a feature flag to enable it

You mean enable printlns if tracing is disabled? I think it's a bit of an overkill to be honest - can you imaging anyone having this flag turned on in their dev-dependencies? Why would they? If you want those printlns for whatever reason, it's a temporary thing and not a permanent one. Temporary things are usually enabled either at runtime or via env vars; permanent ones via cargo manifests. Maybe more suitable is an environment variable then instead of a feature flag?

@aldanor
Copy link
Author

aldanor commented Sep 14, 2022

@la10736 Wonder if there's any further thoughts on this? Or is any help with PR needed?

@la10736
Copy link
Owner

la10736 commented Sep 14, 2022 via email

@la10736
Copy link
Owner

la10736 commented Oct 23, 2022

Ok, you've convinced me. I'll just remove it if there isn't any trace.

la10736 added a commit that referenced this issue Oct 23, 2022
when we trace some arguments.
@la10736
Copy link
Owner

la10736 commented Oct 23, 2022

Done.

@la10736 la10736 closed this as completed Oct 23, 2022
@aldanor
Copy link
Author

aldanor commented Oct 23, 2022

Done.

Thanks! 👍

@Mart-Bogdan
Copy link

It still shows to me. No trace flags was provided. or #[case] is trace, and I didn't understand correctly what it meant by it?

@Mart-Bogdan
Copy link

Oh it is unreleased yet. I see. So can live with this output for now, and wait for release.

Thanks!

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

No branches or pull requests

3 participants