From b3713156067c24d12fdec03b8fe3a804a3af3815 Mon Sep 17 00:00:00 2001 From: Thomas Becker Date: Tue, 22 May 2012 18:16:12 +0200 Subject: [PATCH 1/2] make spdy tests more reliable: fix race condition with goAway frame Change-Id: I3bbdb8eee4a12f082f83730209bd0f8cf2fe7d03 --- .../eclipse/jetty/spdy/ClosedStreamTest.java | 10 +++++++++ .../jetty/spdy/ProtocolViolationsTest.java | 22 +++++++++---------- 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/jetty-spdy/spdy-jetty/src/test/java/org/eclipse/jetty/spdy/ClosedStreamTest.java b/jetty-spdy/spdy-jetty/src/test/java/org/eclipse/jetty/spdy/ClosedStreamTest.java index c0c6a01e053..7b399f35f1e 100644 --- a/jetty-spdy/spdy-jetty/src/test/java/org/eclipse/jetty/spdy/ClosedStreamTest.java +++ b/jetty-spdy/spdy-jetty/src/test/java/org/eclipse/jetty/spdy/ClosedStreamTest.java @@ -26,6 +26,7 @@ import java.util.concurrent.TimeUnit; import org.eclipse.jetty.spdy.api.BytesDataInfo; import org.eclipse.jetty.spdy.api.DataInfo; +import org.eclipse.jetty.spdy.api.GoAwayInfo; import org.eclipse.jetty.spdy.api.Headers; import org.eclipse.jetty.spdy.api.ReplyInfo; import org.eclipse.jetty.spdy.api.SPDY; @@ -180,6 +181,7 @@ public class ClosedStreamTest extends AbstractTest final CountDownLatch serverReplySentLatch = new CountDownLatch(1); final CountDownLatch clientReplyReceivedLatch = new CountDownLatch(1); final CountDownLatch serverDataReceivedLatch = new CountDownLatch(1); + final CountDownLatch goAwayReceivedLatch = new CountDownLatch(1); InetSocketAddress startServer = startServer(new ServerSessionFrameListener.Adapter() { @@ -206,6 +208,11 @@ public class ClosedStreamTest extends AbstractTest } }; } + @Override + public void onGoAway(Session session, GoAwayInfo goAwayInfo) + { + goAwayReceivedLatch.countDown(); + } }); final Generator generator = new Generator(new StandardByteBufferPool(),new StandardCompressionFactory().newCompressor()); @@ -214,6 +221,7 @@ public class ClosedStreamTest extends AbstractTest final SocketChannel socketChannel = SocketChannel.open(startServer); socketChannel.write(synData); + assertThat("synData is fully written", synData.hasRemaining(), is(false)); assertThat("server: syn reply is sent",serverReplySentLatch.await(5,TimeUnit.SECONDS),is(true)); @@ -263,6 +271,8 @@ public class ClosedStreamTest extends AbstractTest socketChannel.write(buffer); Assert.assertThat(buffer.hasRemaining(), is(false)); + assertThat("GoAway frame is received by server", goAwayReceivedLatch.await(5,TimeUnit.SECONDS), is(true)); + socketChannel.close(); } } diff --git a/jetty-spdy/spdy-jetty/src/test/java/org/eclipse/jetty/spdy/ProtocolViolationsTest.java b/jetty-spdy/spdy-jetty/src/test/java/org/eclipse/jetty/spdy/ProtocolViolationsTest.java index eb75be7e078..791b6906770 100644 --- a/jetty-spdy/spdy-jetty/src/test/java/org/eclipse/jetty/spdy/ProtocolViolationsTest.java +++ b/jetty-spdy/spdy-jetty/src/test/java/org/eclipse/jetty/spdy/ProtocolViolationsTest.java @@ -1,5 +1,8 @@ package org.eclipse.jetty.spdy; +import static org.junit.Assert.*; +import static org.hamcrest.Matchers.*; + import java.net.InetSocketAddress; import java.nio.ByteBuffer; import java.nio.channels.ServerSocketChannel; @@ -15,7 +18,6 @@ import org.eclipse.jetty.spdy.api.RstInfo; import org.eclipse.jetty.spdy.api.SPDY; import org.eclipse.jetty.spdy.api.Session; import org.eclipse.jetty.spdy.api.SessionFrameListener; -import org.eclipse.jetty.spdy.api.SessionStatus; import org.eclipse.jetty.spdy.api.Stream; import org.eclipse.jetty.spdy.api.StreamFrameListener; import org.eclipse.jetty.spdy.api.StreamStatus; @@ -23,7 +25,6 @@ import org.eclipse.jetty.spdy.api.StringDataInfo; import org.eclipse.jetty.spdy.api.SynInfo; import org.eclipse.jetty.spdy.api.server.ServerSessionFrameListener; import org.eclipse.jetty.spdy.frames.ControlFrameType; -import org.eclipse.jetty.spdy.frames.GoAwayFrame; import org.eclipse.jetty.spdy.frames.SynReplyFrame; import org.eclipse.jetty.spdy.generator.Generator; import org.junit.Assert; @@ -85,6 +86,7 @@ public class ProtocolViolationsTest extends AbstractTest byte[] bytes = new byte[1]; ByteBuffer writeBuffer = generator.data(streamId, bytes.length, new BytesDataInfo(bytes, true)); channel.write(writeBuffer); + assertThat("data is fully written", writeBuffer.hasRemaining(),is(false)); readBuffer.clear(); channel.read(readBuffer); @@ -92,11 +94,8 @@ public class ProtocolViolationsTest extends AbstractTest Assert.assertEquals(ControlFrameType.RST_STREAM.getCode(), readBuffer.getShort(2)); Assert.assertEquals(streamId, readBuffer.getInt(8)); - writeBuffer = generator.control(new GoAwayFrame(SPDY.V2, 0, SessionStatus.OK.getCode())); - channel.write(writeBuffer); - channel.shutdownOutput(); - channel.close(); - + session.goAway().get(5,TimeUnit.SECONDS); + server.close(); } @@ -129,7 +128,6 @@ public class ProtocolViolationsTest extends AbstractTest @Override public void onData(Stream stream, DataInfo dataInfo) { - System.out.println("ondata"); dataLatch.countDown(); } }); @@ -144,21 +142,21 @@ public class ProtocolViolationsTest extends AbstractTest ByteBuffer writeBuffer = generator.control(new SynReplyFrame(SPDY.V2, (byte)0, streamId, new Headers())); channel.write(writeBuffer); + assertThat("SynReply is fully written", writeBuffer.hasRemaining(), is(false)); byte[] bytes = new byte[1]; writeBuffer = generator.data(streamId, bytes.length, new BytesDataInfo(bytes, true)); channel.write(writeBuffer); + assertThat("data is fully written", writeBuffer.hasRemaining(), is(false)); // Write again to simulate the faulty condition writeBuffer.flip(); channel.write(writeBuffer); + assertThat("data is fully written", writeBuffer.hasRemaining(), is(false)); Assert.assertFalse(dataLatch.await(1, TimeUnit.SECONDS)); - writeBuffer = generator.control(new GoAwayFrame(SPDY.V2, 0, SessionStatus.OK.getCode())); - channel.write(writeBuffer); - channel.shutdownOutput(); - channel.close(); + session.goAway().get(5,TimeUnit.SECONDS); server.close(); } From 32f8900040cd65d84e96c659769bf893ec0ad951 Mon Sep 17 00:00:00 2001 From: Jesse McConnell Date: Tue, 22 May 2012 13:57:58 -0500 Subject: [PATCH 2/2] fix broken unit test because getting the last modified time is a time zone dependent thing and we don't _all_ live in germany and italy --- .../eclipse/jetty/util/resource/ResourceTest.java | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/jetty-util/src/test/java/org/eclipse/jetty/util/resource/ResourceTest.java b/jetty-util/src/test/java/org/eclipse/jetty/util/resource/ResourceTest.java index 571e51a7e5d..3e305a7e1d5 100644 --- a/jetty-util/src/test/java/org/eclipse/jetty/util/resource/ResourceTest.java +++ b/jetty-util/src/test/java/org/eclipse/jetty/util/resource/ResourceTest.java @@ -24,11 +24,15 @@ import java.io.FilenameFilter; import java.io.InputStream; import java.net.URI; import java.net.URL; +import java.sql.Time; import java.util.Arrays; import java.util.Date; import java.util.HashSet; import java.util.Set; +import java.util.TimeZone; +import java.util.zip.ZipFile; +import org.eclipse.jetty.toolchain.test.MavenTestingUtils; import org.eclipse.jetty.toolchain.test.OS; import org.eclipse.jetty.util.IO; import org.junit.BeforeClass; @@ -317,8 +321,14 @@ public class ResourceTest throws Exception { String s = "jar:"+__userURL+"TestData/test.zip!/subdir/numbers"; + + // TODO move this into src/test/resources!!! + ZipFile zf = new ZipFile(MavenTestingUtils.getProjectFile("src/test/java/org/eclipse/jetty/util/resource/TestData/test.zip")); + + long last = zf.getEntry("subdir/numbers").getTime(); + Resource r = Resource.newResource(s); - assertEquals(971425252000L,r.lastModified()); // Known date value inside zip + assertEquals(last,r.lastModified()); // Known date value inside zip } /* ------------------------------------------------------------ */