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

StdOutputStream final linefeed ambiguity. #750

Closed
bkuker opened this issue Aug 30, 2023 · 5 comments · Fixed by #754
Closed

StdOutputStream final linefeed ambiguity. #750

bkuker opened this issue Aug 30, 2023 · 5 comments · Fixed by #754

Comments

@bkuker
Copy link

bkuker commented Aug 30, 2023

StdOutputStream's capturedLines() method takes the string output and splits it by the system line separator, returning an array.

The returned array will be identical whether or not the final line written to standard out has a newline at the end, as String.split treats the split character and the end of the string the same.

You therefore can not test that the final line written to STDOUT ends with a linefeed.

This is a nitpick but I suppose it could matter to someone.

I'd suggest simply adding an additional public String capturedString() method that returns the entire output unchanged.

@nipafx
Copy link
Member

nipafx commented Aug 30, 2023

We took a look at this and you're right. This is the default behavior of String::split: Unless you pass a sufficiently high (or negative) limit trailing empty split results are discarded. Possible fixes:

  • offer a capturedString() method as you describe
  • change capturedLines() to contain trailing empty lines
  • offer that behavior as an overload of capturedLines() (e.g. with a boolean flag)
  • offer that behavior as a new method capturedLinesWithTrailingEmptyBecauseLongMethodNamesAreGreat()

We'll be picking one soon.

@Michael1993
Copy link
Member

After a bit of poking around, I'm going to say it's best if we change how capturedLines works.
Not only does it automatically trim trailing empty strings, it also returns an empty string when it should return an empty array (i.e.: nothing was written to Stdout).

@nipafx
Copy link
Member

nipafx commented Oct 29, 2023

We'll be picking one soon.

... he said two months ago. But we're making progress, @bkuker, and I'm positive we have something for you before November runs out. 😬

@bkuker
Copy link
Author

bkuker commented Oct 31, 2023

Hey, the wheels grind slowly, but they grind fine! Thanks!

@nipafx
Copy link
Member

nipafx commented Nov 7, 2023

Behavior change in StdIo

The bug-fix merged by #754 changes the behavior of the method capturedLines() on StdIn, StdOut, and StdErr.
The returned array now contains trailing empty lines if these were read/printed.

So for code like...

System.out.println();
System.out.println("JUnit Pioneer 🚀");
System.out.println();

... capturedLines looks differently:

  • before: ["", "JUnit Pioneer 🚀"]
  • now: ["", "JUnit Pioneer 🚀", ""]

If code only reads/prints empty lines, these will now also be correctly reported.

Existing assertions based on capturedLines may have to be adapted to this change.

Note that capturedLines still can't distinguish between output that ended with print vs println. The newly added method capturedString provides that capability. For more details on these two methods, see the updated documentation.

nipafx pushed a commit that referenced this issue Nov 7, 2023
The method `capturedLines()` on `StdIn`, `StdOut`, and `StdErr` now
includes trailing empty lines. So for code like...

```java
System.out.println();
System.out.println("JUnit Pioneer 🚀");
System.out.println();
```

... `capturedLines` looks differently:

* before: `["", "JUnit Pioneer 🚀"]`
* now: `["", "JUnit Pioneer 🚀", ""]`

If code only reads/prints empty lines, these will now also be
correctly reported.

Note that `capturedLines` still can't distinguish between output that
ended with `print` vs `println`. The newly added method
`capturedString` provides that capability.

Closes: #750 
PR: #754
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.

3 participants