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

OutputCapture reset method is package-private #33151

Closed
vrumvrumich opened this issue Nov 14, 2022 · 9 comments
Closed

OutputCapture reset method is package-private #33151

vrumvrumich opened this issue Nov 14, 2022 · 9 comments
Labels
status: declined A suggestion or change that we don't feel we should currently apply

Comments

@vrumvrumich
Copy link

vrumvrumich commented Nov 14, 2022

Hello, I'm currently trying to utilize OutputCapture in my unit testing to test logging level. And I face issue that I'm not able to reset OutputCapture, but according this pull request this functionality should be implemented. My spring-boot version is 2.7.4.
Probably, I can change fixture of test every time by using @TestInstance(TestInstance.Lifecycle.PER_METHOD), but I suppose that is not the best solution.
I suppose, adding any functionality to check logging level would be a very good contributing as well.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Nov 14, 2022
@wilkinsona
Copy link
Member

wilkinsona commented Nov 14, 2022

OutputCapture is package-private. Did you mean CapturedOutput?

It looks like support for resetting the captured output was lost amid some of the changes for (#14738, #17029, etc). We can certainly consider reinstating it in some form.

Probably, I can change fixture of test every time by using @testinstance(TestInstance.Lifecycle.PER_METHOD), but I suppose that is not the best solution.

What test framework are you using? I believe JUnit (4 and 5) uses a per-method lifecycle for each test instance by default.

@wilkinsona wilkinsona added the status: waiting-for-feedback We need additional information before we can continue label Nov 14, 2022
@vrumvrumich
Copy link
Author

@wilkinsona Actually, I mean reset method is package-private. Yeah, you are right by default per-method lifecycle is being used, but as far as I use dependency injection for injection CapturedOutput as method parameter, so state of object is not being wiped.
Probably, we need to move this feature back to make such resetting clear. And I assume a good idea to add method to check logging level.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Nov 14, 2022
@vrumvrumich
Copy link
Author

vrumvrumich commented Nov 14, 2022

@wilkinsona Probably, this is not too much to implement. I would be grateful to contribute a little. (using this guide)

@wilkinsona
Copy link
Member

Actually, I mean reset method is package-private. Yeah, you are right by default per-method lifecycle is being used, but as far as I use dependency injection for injection CapturedOutput as method parameter

Ok, you're using CapturedOutput. That's what I really needed to know. The reset() method only exists on OutputCapture at the moment and the whole class is package-private. Just making the method public won't help. It would needed to be added to the CapturedOutput interface as well. Technically, that's a breaking change so we'll have to consider it carefully.

I use dependency injection for injection CapturedOutput as method parameter

Due to the pushing and popping performed by OutputCaptureExtension, one test should not be able to see output that was produced by another test. It sounds like that may not be working for you so I think we need to take a step back. Can you please provide a minimal example that reproduces the problem that you're facing, clearly showing where you'd like the output to have been reset.

@wilkinsona wilkinsona added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Nov 14, 2022
@vrumvrumich
Copy link
Author

@wilkinsona Actually, I have checked this behavior and you are right one test not see logs of another one. But in method being tested I log multiple messages with different log level and it would be so helpful to have method to check message in array view not as string one.
According to this interface CapturedOutput we have methods that just return string

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Nov 17, 2022
@wilkinsona
Copy link
Member

Can you please expand a bit on what you mean by "array view"?

@wilkinsona wilkinsona added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Nov 17, 2022
@vrumvrumich
Copy link
Author

vrumvrumich commented Nov 17, 2022

@wilkinsona OutputCapture has private field systemCaptures of Deque type. CapturedOutput interface have no any access method to this field. I don't want to assert string with every log line by matching it with any regex or using mockito assetrContains on string. I want check collection of elements where each of element represent one log line. Such representation will help to asset for example with Kluent

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Nov 17, 2022
@philwebb
Copy link
Member

philwebb commented Nov 18, 2022

systemCaptures holds a collection of SystemCapture instance, but these don't necessarily relate to lines. They are rather a block of captured bytes (which may or may not be a complete line). I think we should keep this internal to the class.

If you want to to do per line assertions you can do .spit(...) on CapturedOutput.getAll()

@philwebb philwebb closed this as not planned Won't fix, can't repro, duplicate, stale Nov 18, 2022
@philwebb philwebb added status: declined A suggestion or change that we don't feel we should currently apply and removed status: waiting-for-triage An issue we've not yet triaged status: feedback-provided Feedback has been provided labels Nov 18, 2022
@wilkinsona
Copy link
Member

If you're using Java 11 or later, java.lang.String.lines() is another option for getting each line from the captured output.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Development

No branches or pull requests

4 participants