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

Add hpack-test-case based tests #976

Merged
merged 1 commit into from Jul 18, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitmodules
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[submodule "okhttp-hpacktests/src/test/resources/hpack-test-case"]
path = okhttp-hpacktests/src/test/resources/hpack-test-case
url = git://github.com/http2jp/hpack-test-case.git
22 changes: 22 additions & 0 deletions okhttp-hpacktests/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
OkHttp HPACK tests
==================

These tests use the [hpack-test-case][1] project to validate OkHttp's HPACK
implementation. The HPACK test cases are in a separate git submodule, so to
initialize them, you must run:

git submodule init
git submodule update

When new interop tests are available, you should update
HpackDecodeInteropGoodTest#GOOD_INTEROP_TESTS with the directory name.

TODO
----

* Add maven goal to avoid manual call to git submodule init.
* Make hpack-test-case update itself from git, and run new tests.
* Add maven goal to generate stories and a pull request to hpack-test-case
to have others validate our output.

[1]: https://github.com/http2jp/hpack-test-case
57 changes: 57 additions & 0 deletions okhttp-hpacktests/pom.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
<?xml version="1.0" encoding="UTF-8"?>

<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
<modelVersion>4.0.0</modelVersion>

<parent>
<groupId>com.squareup.okhttp</groupId>
<artifactId>parent</artifactId>
<version>2.0.1-SNAPSHOT</version>
</parent>

<artifactId>okhttp-hpacktests</artifactId>
<name>OkHttp HPACK Tests</name>

<dependencies>
<dependency>
<groupId>com.squareup.okio</groupId>
<artifactId>okio</artifactId>
</dependency>
<dependency>
<groupId>com.squareup.okhttp</groupId>
<artifactId>okhttp</artifactId>
<version>${project.version}</version>
</dependency>
<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.squareup.okhttp</groupId>
<artifactId>mockwebserver</artifactId>
<version>${project.version}</version>
<scope>test</scope>
</dependency>
<!-- Gson: Java to Json conversion -->
<dependency>
<groupId>com.google.code.gson</groupId>
<artifactId>gson</artifactId>
<version>2.2.4</version>
<scope>compile</scope>
</dependency>
</dependencies>

<build>
<plugins>
<!-- Do not deploy this as an artifact to Maven central. -->
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-deploy-plugin</artifactId>
<configuration>
<skip>true</skip>
</configuration>
</plugin>
</plugins>
</build>
</project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/*
* Copyright (C) 2014 Square, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.squareup.okhttp.internal.spdy;

import com.squareup.okhttp.internal.spdy.hpackjson.Story;
import org.junit.Ignore;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;

import java.util.Collection;

/**
* Known bad tests for HPACK interop.
*/
// TODO: fix these tests (see if the input/test is legit, fix the implementation.)
@Ignore
@RunWith(Parameterized.class)
public class HpackDecodeInteropBadTest extends HpackDecodeTestBase {

private static final String[] BAD_INTEROP_TESTS = { "go-hpack", "haskell-http2-diff-huffman",
"haskell-http2-linear-huffman", "haskell-http2-naive-huffman",
"haskell-http2-static-huffman", "node-http2-protocol", "twitter-hpack" };

public HpackDecodeInteropBadTest(Story story) {
super(story);
}

@Parameterized.Parameters(name="{0}")
public static Collection<Story[]> createStories() throws Exception {
return createStories(BAD_INTEROP_TESTS);
}

@Test
public void testGoodDecoderInterop() throws Exception {
testDecoder();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/*
* Copyright (C) 2014 Square, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.squareup.okhttp.internal.spdy;

import com.squareup.okhttp.internal.spdy.hpackjson.Story;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;

import java.util.Collection;

/**
* Known good tests for HPACK interop.
*/
@RunWith(Parameterized.class)
public class HpackDecodeInteropGoodTest extends HpackDecodeTestBase {


private static final String[] GOOD_INTEROP_TESTS = { "haskell-http2-diff",
"haskell-http2-linear", "haskell-http2-naive", "haskell-http2-static",
"hyper-hpack", "nghttp2", "nghttp2-16384-4096",
"nghttp2-change-table-size", "node-http2-hpack" };

public HpackDecodeInteropGoodTest(Story story) {
super(story);
}

@Parameterized.Parameters(name="{0}")
public static Collection<Story[]> createStories() throws Exception {
return createStories(GOOD_INTEROP_TESTS);
}

@Test
public void testGoodDecoderInterop() throws Exception {
testDecoder();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
/*
* Copyright (C) 2014 Square, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.squareup.okhttp.internal.spdy;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.fail;

import com.squareup.okhttp.internal.spdy.hpackjson.Case;
import com.squareup.okhttp.internal.spdy.hpackjson.HpackJsonUtil;
import com.squareup.okhttp.internal.spdy.hpackjson.Story;
import okio.Buffer;

import java.util.ArrayList;
import java.util.Collection;
import java.util.LinkedHashSet;
import java.util.List;

/**
* Tests Hpack implementation using https://github.com/http2jp/hpack-test-case/
*/
public class HpackDecodeTestBase {

/**
* Reads all stories in the folders provided, asserts if no story found.
*/
protected static Collection<Story[]> createStories(String[] interopTests)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make sense to flatten the collection?
Otherwise, why not a Collection<List> ?
(I don't like arrays)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Parameterized.class requires arrays for passing to the constructor :|

each one has one item in it. Added comment.

throws Exception {
List<Story[]> result = new ArrayList<>();
for (String interopTestName : interopTests) {
List<Story> stories = HpackJsonUtil.readStories(interopTestName);
if (stories.isEmpty()) {
fail("No stories for: " + interopTestName);
}
for (Story story : stories) {
result.add(new Story[] { story });
}
}
return result;
}

private final Buffer bytesIn = new Buffer();
private final HpackDraft08.Reader hpackReader = new HpackDraft08.Reader(4096, bytesIn);

private final Story story;

public HpackDecodeTestBase(Story story) {
this.story = story;
}

/**
* Expects wire to be set for all cases, and compares the decoder's output to
* expected headers.
*/
protected void testDecoder() throws Exception {
testDecoder(story);
}

protected void testDecoder(Story story) throws Exception {
for (Case caze : story.getCases()) {
bytesIn.write(caze.getWire());
hpackReader.readHeaders();
hpackReader.emitReferenceSet();
assertSetEquals(String.format("seqno=%d", caze.getSeqno()), caze.getHeaders(),
hpackReader.getAndReset());
}
}
/**
* Checks if {@code expected} and {@code observed} are equal when viewed as a
* set and headers are deduped.
*
* TODO: See if duped headers should be preserved on decode and verify.
*/
private static void assertSetEquals(
String message, List<Header> expected, List<Header> observed) {
assertEquals(message, new LinkedHashSet<>(expected), new LinkedHashSet<>(observed));
}

protected Story getStory() {
return story;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
/*
* Copyright (C) 2014 Square, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.squareup.okhttp.internal.spdy;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: copyright header

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done?


import com.squareup.okhttp.internal.spdy.hpackjson.Case;
import com.squareup.okhttp.internal.spdy.hpackjson.Story;
import okio.Buffer;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;

import java.util.Collection;

/**
* Tests for round-tripping headers through hpack..
*/
// TODO: update hpack-test-case with the output of our encoder.
// This test will hide complementary bugs in the encoder and decoder,
// We should test that the encoder is producing responses that are
// d]
@RunWith(Parameterized.class)
public class HpackRoundTripTest extends HpackDecodeTestBase {

private static final String[] RAW_DATA = { "raw-data" };

@Parameterized.Parameters(name="{0}")
public static Collection<Story[]> getStories() throws Exception {
return createStories(RAW_DATA);
}

private Buffer bytesOut = new Buffer();
private HpackDraft08.Writer hpackWriter = new HpackDraft08.Writer(bytesOut);

public HpackRoundTripTest(Story story) {
super(story);
}

@Test
public void testRoundTrip() throws Exception {
Story story = getStory().clone();
// Mutate cases in base class.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe instead, you could add an overload of testDecoder that takes a parameter?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

for (Case caze : story.getCases()) {
hpackWriter.writeHeaders(caze.getHeaders());
caze.setWire(bytesOut.readByteString());
}

testDecoder(story);
}

}
Loading