Skip to content

Commit 8c217f5

Browse files
committed
Replace third test to cover flush-specific behavior
Old test was misleading - it toggled isReady but buffering was triggered by readyAndDrained=false (from first write), not isReady state. New test 'flush_isReadyTrue_goesDirect_isReadyFalse_buffers' tests the flush path in runOrBuffer(): - Flush with isReady=true: goes direct (readyAndDrained stays true) - Flush with isReady=false: buffers (readyAndDrained becomes false) This directly validates the flush behavior preservation mentioned in the PR. Reviewed-by: copilot-pull-request-reviewer[bot] Fixes #12790
1 parent e898755 commit 8c217f5

1 file changed

Lines changed: 20 additions & 18 deletions

File tree

servlet/src/test/java/io/grpc/servlet/AsyncServletOutputStreamWriterTest.java

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,9 @@
3333
/**
3434
* Unit test for {@link AsyncServletOutputStreamWriter} with a mock isReady supplier.
3535
* Tests three scenarios:
36-
* (1) A write followed by another write without onWritePossible() is buffered.
36+
* (1) A write without intervening onWritePossible() is buffered.
3737
* (2) Consecutive writes with onWritePossible() between them succeed.
38-
* (3) When isReady() becomes false, writes are buffered until onWritePossible() drains them.
38+
* (3) Flush behavior: goes direct when isReady() is true, buffers when isReady() is false.
3939
*/
4040
@RunWith(JUnit4.class)
4141
public class AsyncServletOutputStreamWriterTest {
@@ -124,11 +124,12 @@ public void writeBytes_onWritePossibleBetweenWrites_succeeds() throws IOExceptio
124124
}
125125

126126
/**
127-
* Test that when isReady() returns false after a direct write, subsequent writes
128-
* are buffered and only drain when onWritePossible() is called.
127+
* Test flush behavior: flush keeps readyAndDrained=true when isReady() stays true
128+
* (goes direct), but transitions to buffering when isReady() becomes false.
129+
* This covers the flush-specific path in runOrBuffer().
129130
*/
130131
@Test
131-
public void writeBytes_isReadyFalse_buffersUntilOnWritePossible() throws IOException {
132+
public void flush_isReadyTrue_goesDirect_isReadyFalse_buffers() throws IOException {
132133
AtomicBoolean isReady = new AtomicBoolean(true);
133134
List<String> actions = new ArrayList<>();
134135

@@ -151,23 +152,24 @@ public void writeBytes_isReadyFalse_buffersUntilOnWritePossible() throws IOExcep
151152
// Initial onWritePossible to set readyAndDrained=true
152153
writer.onWritePossible();
153154

154-
// First write - goes direct, readyAndDrained becomes false
155-
byte[] data1 = new byte[]{1};
156-
writer.writeBytes(data1, 1);
155+
// Flush with isReady=true - should go direct (readyAndDrained stays true)
156+
writer.flush();
157+
assertEquals("Flush should execute directly when isReady=true", 1, actions.size());
157158

158-
// After first write, readyAndDrained=false, so any subsequent write is buffered.
159-
// This buffering happens regardless of isReady() state (the Tomcat fix).
160-
// Second write - should be buffered since readyAndDrained=false
161-
byte[] data2 = new byte[]{2};
162-
writer.writeBytes(data2, 1);
159+
// Simulate isReady becoming false (container buffer full)
160+
isReady.set(false);
161+
162+
// Next flush - should be buffered since isReady=false
163+
writer.flush();
163164

164-
// Only the first write should have executed
165-
assertEquals("First write should complete, second buffered", 1, actions.size());
165+
// Only the first flush should have executed
166+
assertEquals("Second flush should be buffered when isReady=false", 1, actions.size());
166167

167-
// Container calls onWritePossible to drain buffered writes
168+
// Container calls onWritePossible to drain buffered flush
169+
isReady.set(true);
168170
writer.onWritePossible();
169171

170-
// Now both writes should have completed
171-
assertEquals("Both writes should complete after onWritePossible", 2, actions.size());
172+
// Now both flushes should have completed
173+
assertEquals("Both flushes should complete after onWritePossible", 2, actions.size());
172174
}
173175
}

0 commit comments

Comments
 (0)