Skip to content

Commit ccb13ee

Browse files
committed
Fix flush test per Copilot review
Test was incorrectly setting isReady=false before second flush, but flushAction runs BEFORE the isReady check in runOrBuffer(), so the flush would still execute directly. New approach: use writeBytes to set readyAndDrained=false, then call flush. Since readyAndDrained=false, flush is buffered. Renamed test to flush_withReadyAndDrainedFalse_isBuffered to match what it actually tests. Reviewed-by: copilot-pull-request-reviewer[bot] Fixes #12790
1 parent 8c217f5 commit ccb13ee

1 file changed

Lines changed: 13 additions & 12 deletions

File tree

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

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@
3535
* Tests three scenarios:
3636
* (1) A write without intervening onWritePossible() is buffered.
3737
* (2) Consecutive writes with onWritePossible() between them succeed.
38-
* (3) Flush behavior: goes direct when isReady() is true, buffers when isReady() is false.
38+
* (3) Flush goes direct when readyAndDrained=true, buffers when readyAndDrained=false.
3939
*/
4040
@RunWith(JUnit4.class)
4141
public class AsyncServletOutputStreamWriterTest {
@@ -124,12 +124,12 @@ public void writeBytes_onWritePossibleBetweenWrites_succeeds() throws IOExceptio
124124
}
125125

126126
/**
127-
* Test flush behavior: flush keeps readyAndDrained=true when isReady() stays true
128-
* (goes direct), but transitions to buffering when isReady() becomes false.
127+
* Test flush behavior: flush goes direct when readyAndDrained is true,
128+
* but is buffered when readyAndDrained is false (regardless of isReady state).
129129
* This covers the flush-specific path in runOrBuffer().
130130
*/
131131
@Test
132-
public void flush_isReadyTrue_goesDirect_isReadyFalse_buffers() throws IOException {
132+
public void flush_withReadyAndDrainedFalse_isBuffered() throws IOException {
133133
AtomicBoolean isReady = new AtomicBoolean(true);
134134
List<String> actions = new ArrayList<>();
135135

@@ -152,24 +152,25 @@ public void flush_isReadyTrue_goesDirect_isReadyFalse_buffers() throws IOExcepti
152152
// Initial onWritePossible to set readyAndDrained=true
153153
writer.onWritePossible();
154154

155-
// Flush with isReady=true - should go direct (readyAndDrained stays true)
155+
// First flush - readyAndDrained=true, isReady=true -> goes direct
156156
writer.flush();
157-
assertEquals("Flush should execute directly when isReady=true", 1, actions.size());
157+
assertEquals("First flush should execute directly", 1, actions.size());
158158

159-
// Simulate isReady becoming false (container buffer full)
160-
isReady.set(false);
159+
// Write a byte to set readyAndDrained=false (writeBytes always clears it)
160+
writer.writeBytes(new byte[]{1}, 1);
161+
// The write goes direct (readyAndDrained was true) and readyAndDrained becomes false.
162+
// Now readyAndDrained=false.
161163

162-
// Next flush - should be buffered since isReady=false
164+
// Flush with readyAndDrained=false -> should be buffered (isReady doesn't matter)
163165
writer.flush();
164166

165167
// Only the first flush should have executed
166-
assertEquals("Second flush should be buffered when isReady=false", 1, actions.size());
168+
assertEquals("Second flush should be buffered", 1, actions.size());
167169

168170
// Container calls onWritePossible to drain buffered flush
169-
isReady.set(true);
170171
writer.onWritePossible();
171172

172-
// Now both flushes should have completed
173+
// Both flushes should have completed
173174
assertEquals("Both flushes should complete after onWritePossible", 2, actions.size());
174175
}
175176
}

0 commit comments

Comments
 (0)