From e917bd0ed00567b39878decd3fbfd6b35245a3eb Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Mon, 3 Oct 2022 16:55:44 -0700 Subject: [PATCH] Only build OutputCapture strings when necessary Update `OutputCapture` so that expensive build operations are only performed when necessary. This update is especially important for Kotlin users calling `"Expected String" in output` since this results in a call to Kotlin's `CharSequence.contains` which calls the `length()` and `charAt()` methods many times. Closes gh-32033 --- .../boot/test/system/OutputCapture.java | 53 +++++++++++--- .../boot/test/system/OutputCaptureTests.java | 72 +++++++++++++++---- 2 files changed, 100 insertions(+), 25 deletions(-) diff --git a/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/system/OutputCapture.java b/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/system/OutputCapture.java index f35a610df8b9..37804a042dbb 100644 --- a/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/system/OutputCapture.java +++ b/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/system/OutputCapture.java @@ -23,6 +23,7 @@ import java.util.ArrayList; import java.util.Deque; import java.util.List; +import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; import java.util.function.Predicate; @@ -48,6 +49,12 @@ class OutputCapture implements CapturedOutput { private AnsiOutputState ansiOutputState; + private final AtomicReference out = new AtomicReference<>(null); + + private final AtomicReference err = new AtomicReference<>(null); + + private final AtomicReference all = new AtomicReference<>(null); + /** * Push a new system capture session onto the stack. */ @@ -55,13 +62,15 @@ final void push() { if (this.systemCaptures.isEmpty()) { this.ansiOutputState = AnsiOutputState.saveAndDisable(); } - this.systemCaptures.addLast(new SystemCapture()); + clearExisting(); + this.systemCaptures.addLast(new SystemCapture(this::clearExisting)); } /** * Pop the last system capture session from the stack. */ final void pop() { + clearExisting(); this.systemCaptures.removeLast().release(); if (this.systemCaptures.isEmpty() && this.ansiOutputState != null) { this.ansiOutputState.restore(); @@ -97,7 +106,7 @@ public String toString() { */ @Override public String getAll() { - return get((type) -> true); + return get(this.all, (type) -> true); } /** @@ -106,7 +115,7 @@ public String getAll() { */ @Override public String getOut() { - return get(Type.OUT::equals); + return get(this.out, Type.OUT::equals); } /** @@ -115,19 +124,35 @@ public String getOut() { */ @Override public String getErr() { - return get(Type.ERR::equals); + return get(this.err, Type.ERR::equals); } /** * Resets the current capture session, clearing its captured output. */ void reset() { + clearExisting(); this.systemCaptures.peek().reset(); } - private String get(Predicate filter) { + void clearExisting() { + this.out.set(null); + this.err.set(null); + this.all.set(null); + } + + private String get(AtomicReference existing, Predicate filter) { Assert.state(!this.systemCaptures.isEmpty(), "No system captures found. Please check your output capture registration."); + String result = existing.get(); + if (result == null) { + result = build(filter); + existing.compareAndSet(null, result); + } + return result; + } + + String build(Predicate filter) { StringBuilder builder = new StringBuilder(); for (SystemCapture systemCapture : this.systemCaptures) { systemCapture.append(builder, filter); @@ -141,6 +166,8 @@ private String get(Predicate filter) { */ private static class SystemCapture { + private final Runnable onCapture; + private final Object monitor = new Object(); private final PrintStreamCapture out; @@ -149,7 +176,8 @@ private static class SystemCapture { private final List capturedStrings = new ArrayList<>(); - SystemCapture() { + SystemCapture(Runnable onCapture) { + this.onCapture = onCapture; this.out = new PrintStreamCapture(System.out, this::captureOut); this.err = new PrintStreamCapture(System.err, this::captureErr); System.setOut(this.out); @@ -162,14 +190,17 @@ void release() { } private void captureOut(String string) { - synchronized (this.monitor) { - this.capturedStrings.add(new CapturedString(Type.OUT, string)); - } + capture(new CapturedString(Type.OUT, string)); } private void captureErr(String string) { + capture(new CapturedString(Type.ERR, string)); + } + + private void capture(CapturedString e) { synchronized (this.monitor) { - this.capturedStrings.add(new CapturedString(Type.ERR, string)); + this.onCapture.run(); + this.capturedStrings.add(e); } } @@ -276,7 +307,7 @@ public String toString() { /** * Types of content that can be captured. */ - private enum Type { + enum Type { OUT, ERR diff --git a/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/system/OutputCaptureTests.java b/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/system/OutputCaptureTests.java index a18b398541a3..c6c3ac341082 100644 --- a/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/system/OutputCaptureTests.java +++ b/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/system/OutputCaptureTests.java @@ -19,6 +19,7 @@ import java.io.ByteArrayOutputStream; import java.io.PrintStream; import java.util.NoSuchElementException; +import java.util.function.Predicate; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -42,7 +43,7 @@ class OutputCaptureTests { private TestPrintStream systemErr; - private OutputCapture output = new OutputCapture(); + private TestOutputCapture output = new TestOutputCapture(); @BeforeEach void replaceSystemStreams() { @@ -129,38 +130,69 @@ void subSequenceReturnsCapturedSubSequence() { @Test void getAllReturnsAllCapturedOutput() { - this.output.push(); - System.out.print("A"); - System.err.print("B"); - System.out.print("C"); + pushAndPrint(); assertThat(this.output.getAll()).isEqualTo("ABC"); } @Test void toStringReturnsAllCapturedOutput() { - this.output.push(); - System.out.print("A"); - System.err.print("B"); - System.out.print("C"); + pushAndPrint(); assertThat(this.output.toString()).isEqualTo("ABC"); } @Test void getErrReturnsOnlyCapturedErrOutput() { - this.output.push(); - System.out.print("A"); - System.err.print("B"); - System.out.print("C"); + pushAndPrint(); assertThat(this.output.getErr()).isEqualTo("B"); } @Test void getOutReturnsOnlyCapturedOutOutput() { + pushAndPrint(); + assertThat(this.output.getOut()).isEqualTo("AC"); + } + + @Test + void getAllUsesCache() { + pushAndPrint(); + for (int i = 0; i < 10; i++) { + assertThat(this.output.getAll()).isEqualTo("ABC"); + } + assertThat(this.output.buildCount).isEqualTo(1); + System.out.print("X"); + assertThat(this.output.getAll()).isEqualTo("ABCX"); + assertThat(this.output.buildCount).isEqualTo(2); + } + + @Test + void getOutUsesCache() { + pushAndPrint(); + for (int i = 0; i < 10; i++) { + assertThat(this.output.getOut()).isEqualTo("AC"); + } + assertThat(this.output.buildCount).isEqualTo(1); + System.out.print("X"); + assertThat(this.output.getOut()).isEqualTo("ACX"); + assertThat(this.output.buildCount).isEqualTo(2); + } + + @Test + void getErrUsesCache() { + pushAndPrint(); + for (int i = 0; i < 10; i++) { + assertThat(this.output.getErr()).isEqualTo("B"); + } + assertThat(this.output.buildCount).isEqualTo(1); + System.err.print("X"); + assertThat(this.output.getErr()).isEqualTo("BX"); + assertThat(this.output.buildCount).isEqualTo(2); + } + + private void pushAndPrint() { this.output.push(); System.out.print("A"); System.err.print("B"); System.out.print("C"); - assertThat(this.output.getOut()).isEqualTo("AC"); } static class TestPrintStream extends PrintStream { @@ -176,4 +208,16 @@ public String toString() { } + static class TestOutputCapture extends OutputCapture { + + int buildCount; + + @Override + String build(Predicate filter) { + this.buildCount++; + return super.build(filter); + } + + } + }