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

Introduce --redirect-stdout and --redirect-stderr CLI options to redirect STDOUT and STDERR #3637

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

mobounya
Copy link
Contributor

@mobounya mobounya commented Jan 14, 2024

Overview

Add new CLI options --redirect-stdout and --redirect-stderr to redirect stdout and stderr respectively to a file,
if both are set to the same file, stdout and stderr will be merged using the configuration parameter
CAPTURE_MERGED_STANDARD_STREAMS_PROPERTY_NAME and captured from stdout key STDOUT_REPORT_ENTRY_KEY in a new TestExecutionListener : RedirectStdoutAndStderrListener

Issue: #3166


I hereby agree to the terms of the JUnit Contributor License Agreement.


Definition of Done

@mobounya mobounya changed the title Feat redirect stderr and stdout Add new CLI options --redirect-stdout and --redirect-stderr to redirect stdout and stderr Jan 14, 2024
mobounya added 4 commits January 14, 2024 15:35
Add method registerMergedStandardStreams in StreamInterceptor to merge
stdout and stderr so both can be intercepted in an stdout interceptor.
This will keep the relative order for both outputs, which makes it
easier to correlate error messages with the corresponding output.

Add new configuration parameter junit.platform.output.capture.merge to
merge stdout and stderr and publish it as STDOUT_REPORT_ENTRY_KEY to
all registered TestExecutionListener instances.

Issue: junit-team#3166
Add new CLI options --redirect-stdout and --redirect-stderr to
redirect stdout and stderr to a file, if both are set to the same file,
stdout and stderr will be merged so we can keep the relative order of
messages. This makes it easier to correlate error messages with the
corresponding output.

Issue: junit-team#3166
Document the changes for the previous two commit in the 5.11.0-M1
release notes and in the user guide.

Issue: junit-team#3166
Add @API annotation to new methods.

Issue: junit-team#3166
@mobounya mobounya force-pushed the feat-redirect_stderr_and_stdout branch from 1dc0449 to ae0fce5 Compare January 14, 2024 14:35
@ahmedelbahri

This comment was marked as off-topic.

@sbrannen sbrannen changed the title Add new CLI options --redirect-stdout and --redirect-stderr to redirect stdout and stderr Introduce --redirect-stdout and --redirect-stderr CLI options to redirect STDOUT and STDERR Jan 14, 2024
Copy link
Member

@marcphilipp marcphilipp left a comment

Choose a reason for hiding this comment

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

I don't think we should implement this new ConsoleLauncher feature based on the report entries. Instead, the ConsoleLauncher should replace System.out and System.err prior to calling Launcher.execute with PrintStreams writing to files (or the same file). This would allow to configure capturing the output as report entries separately.


* New optional config parameter `junit.platform.output.capture.merge` that can be used to merge stdout and stderr and
capture it as stdout.
* New optional CLI options `--redirect-stdout` and `--redirect-stderr` to redirect stdout and stderr outputs to a file.
Copy link
Member

Choose a reason for hiding this comment

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

This needs to go to the release notes of 5.12.0-M1 now

@@ -73,4 +75,24 @@ public void setTheme(Theme theme) {
this.theme = theme;
}

@API(status = INTERNAL, since = "5.11")
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
@API(status = INTERNAL, since = "5.11")
@API(status = INTERNAL, since = "5.12")

same below

Comment on lines +57 to +59
@Option(names = "-redirect-stdout", hidden = true)
private Path stdout2;

Copy link
Member

Choose a reason for hiding this comment

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

We're only maintaining this kind of options for backwards compatibility (we switched to option parsers at some point) but we shouldn't introduce them for new options.

Suggested change
@Option(names = "-redirect-stdout", hidden = true)
private Path stdout2;

Comment on lines +63 to +65
@Option(names = "-redirect-stderr", hidden = true)
private Path stderr2;

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
@Option(names = "-redirect-stderr", hidden = true)
private Path stderr2;

Comment on lines +71 to +72
result.setStdoutPath(choose(stdout, stdout2, null));
result.setStderrPath(choose(stderr, stderr2, null));
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
result.setStdoutPath(choose(stdout, stdout2, null));
result.setStderrPath(choose(stderr, stderr2, null));
result.setStdoutPath(stdout);
result.setStderrPath(stderr);

@marcphilipp
Copy link
Member

@mobounya Thank you for your PR! Sorry it took us so long to review it. Are you still interested in continuing?

@marcphilipp
Copy link
Member

@mobounya We'd like to make progress on this issue. Please let us know if you're interested in continuing on this PR.

@mobounya
Copy link
Contributor Author

Sorry for the delay, I've been busy with work, yes I will pick this up in the weekend

@marcphilipp
Copy link
Member

@mobounya Any updates?

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

Successfully merging this pull request may close these issues.

Allow ConsoleLauncher to redirect STDOUT and STDERR to files instead of the console
4 participants