SPDY - unforked spdy_framer and spdy_protocol tests.

net/spdy/spdy_protocol_test.cc

Index: net/spdy/spdy_protocol_test.cc
===================================================================
--- net/spdy/spdy_protocol_test.cc	(working copy)
+++ net/spdy/spdy_protocol_test.cc	(working copy)
@@ -45,10 +45,43 @@
 namespace {
 
 // Default SPDY version for unit tests.
-const int SPDY_VERSION_FOR_TESTS = 3;
+int SPDY_VERSION_FOR_TESTS = 2;

[Ryan Hamilton, 2012/03/13 19:59:12]

Ditto on my advice from the other test about making this a member variable.

[ramant (doing other things), 2012/03/13 21:43:02]

Done.

 
+enum SpdyProtocolTestTypes {
+  SPDY2,
+  SPDY3,
+};
+
+class SpdyProtocolTest
+    : public ::testing::TestWithParam<SpdyProtocolTestTypes> {
+ protected:
+  virtual void SetUp() {
+    SPDY_VERSION_FOR_TESTS = (GetParam() == SPDY2) ? 2 : 3;
+  }
+
+  virtual void TearDown() {}
+
+};
+
+class SpdyProtocolDeathTest

[Ryan Hamilton, 2012/03/13 19:59:12]

I might be missing something, but if this class is identical to
SpdyProtocolTest, is there a reason we need to have 2 different classes?

(And if we really need it to be a distinct class perhaps we can just do:

class SpdyProtocolDeathTest : public SpdyProtocolTest {};

[ramant (doing other things), 2012/03/13 21:43:02]

Done.

+    : public ::testing::TestWithParam<SpdyProtocolTestTypes> {
+ protected:
+  virtual void SetUp() {
+    SPDY_VERSION_FOR_TESTS = (GetParam() == SPDY2) ? 2 : 3;
+  }
+
+  virtual void TearDown() {}
+
+};
+
+//-----------------------------------------------------------------------------
+// All tests are run with two different SPDY versions: SPDY/2 and SPDY/3.
+INSTANTIATE_TEST_CASE_P(SpdyProtocolTests,
+                        SpdyProtocolTest,
+                        ::testing::Values(SPDY2, SPDY3));
+
 // Test our protocol constants
-TEST(SpdyProtocolSpdy3Test, ProtocolConstants) {
+TEST(SpdyProtocolTest, ProtocolConstants) {

[Ryan Hamilton, 2012/03/13 19:59:12]

Can the various TEST()s be replaced with TEST_P()s?

[ramant (doing other things), 2012/03/13 21:43:02]

Done.

   EXPECT_EQ(8u, SpdyFrame::kHeaderSize);
   EXPECT_EQ(8u, SpdyDataFrame::size());
   EXPECT_EQ(8u, SpdyControlFrame::kHeaderSize);
@@ -73,7 +106,7 @@
 }
 
 // Test some of the protocol helper functions
-TEST(SpdyProtocolSpdy3Test, FrameStructs) {
+TEST(SpdyProtocolTest, FrameStructs) {
   SpdyFrame frame(SpdyFrame::kHeaderSize);
   frame.set_length(12345);
   frame.set_flags(10);
@@ -88,13 +121,13 @@
   EXPECT_FALSE(frame.is_control_frame());
 }
 
-TEST(SpdyProtocolSpdy3Test, DataFrameStructs) {
+TEST(SpdyProtocolTest, DataFrameStructs) {
   SpdyDataFrame data_frame;
   data_frame.set_stream_id(12345);
   EXPECT_EQ(12345u, data_frame.stream_id());
 }
 
-TEST(SpdyProtocolSpdy3Test, ControlFrameStructs) {
+TEST_P(SpdyProtocolTest, ControlFrameStructs) {
   SpdyFramer framer(SPDY_VERSION_FOR_TESTS);
   SpdyHeaderBlock headers;
 
@@ -171,7 +204,7 @@
   EXPECT_EQ(456u, window_update_frame->delta_window_size());
 }
 
-TEST(SpdyProtocolSpdy3Test, TestDataFrame) {
+TEST(SpdyProtocolTest, TestDataFrame) {
   SpdyDataFrame frame;
 
   // Set the stream ID to various values.
@@ -214,7 +247,7 @@
 }
 
 // Test various types of SETTINGS frames.
-TEST(SpdyProtocolSpdy3Test, TestSpdySettingsFrame) {
+TEST_P(SpdyProtocolTest, TestSpdySettingsFrame) {
   SpdyFramer framer(SPDY_VERSION_FOR_TESTS);
 
   // Create a settings frame with no settings.
@@ -262,7 +295,7 @@
   }
 }
 
-TEST(SpdyProtocolSpdy3Test, HasHeaderBlock) {
+TEST(SpdyProtocolTest, HasHeaderBlock) {
   SpdyControlFrame frame(SpdyControlFrame::kHeaderSize);
   for (SpdyControlType type = SYN_STREAM;
       type < NUM_CONTROL_FRAME_TYPES;
@@ -276,10 +309,16 @@
   }
 }
 
+//-----------------------------------------------------------------------------
+// All tests are run with two different SPDY versions: SPDY/2 and SPDY/3.
+INSTANTIATE_TEST_CASE_P(SpdyProtocolDeathTests,
+                        SpdyProtocolDeathTest,
+                        ::testing::Values(SPDY2, SPDY3));
+
 // Make sure that overflows both die in debug mode, and do not cause problems
 // in opt mode.  Note:  The EXPECT_DEBUG_DEATH call does not work on Win32 yet,
 // so we comment it out.
-TEST(SpdyProtocolDeathSpdy3Test, TestDataFrame) {
+TEST(SpdyProtocolDeathTest, TestDataFrame) {
   SpdyDataFrame frame;
 
   frame.set_stream_id(0);
@@ -304,7 +343,7 @@
   EXPECT_EQ(0, frame.flags());
 }
 
-TEST(SpdyProtocolDeathSpdy3Test, TestSpdyControlFrameStreamId) {
+TEST(SpdyProtocolDeathTest, TestSpdyControlFrameStreamId) {
   SpdyControlFrame frame_store(SpdySynStreamControlFrame::size());
   memset(frame_store.data(), '1', SpdyControlFrame::kHeaderSize);
   SpdySynStreamControlFrame* frame =
@@ -319,7 +358,7 @@
   EXPECT_FALSE(frame->is_control_frame());
 }
 
-TEST(SpdyProtocolDeathSpdy3Test, TestSpdyControlFrameVersion) {
+TEST(SpdyProtocolDeathTest, TestSpdyControlFrameVersion) {
   const unsigned int kVersionMask = 0x7fff;
   SpdyControlFrame frame(SpdySynStreamControlFrame::size());
   memset(frame.data(), '1', SpdyControlFrame::kHeaderSize);
@@ -340,7 +379,7 @@
   EXPECT_EQ(SYN_STREAM, frame.type());
 }
 
-TEST(SpdyProtocolDeathSpdy3Test, TestSpdyControlFrameType) {
+TEST_P(SpdyProtocolDeathTest, TestSpdyControlFrameType) {
   SpdyControlFrame frame(SpdyControlFrame::kHeaderSize);
   memset(frame.data(), 255, SpdyControlFrame::kHeaderSize);
 
@@ -353,7 +392,7 @@
   for (int i = SYN_STREAM; i <= spdy::WINDOW_UPDATE; ++i) {
     frame.set_type(static_cast<SpdyControlType>(i));
     EXPECT_EQ(i, static_cast<int>(frame.type()));
-    if (i == spdy::NOOP) {
+    if (SPDY_VERSION_FOR_TESTS != 2 && i == spdy::NOOP) {
       // NOOP frames aren't 'valid'.
       EXPECT_FALSE(frame.AppearsToBeAValidControlFrame());
     } else {
@@ -365,7 +404,7 @@
   }
 }
 
-TEST(SpdyProtocolDeathSpdy3Test, TestRstStreamStatusBounds) {
+TEST_P(SpdyProtocolDeathTest, TestRstStreamStatusBounds) {
   SpdyFramer framer(SPDY_VERSION_FOR_TESTS);
   scoped_ptr<SpdyRstStreamControlFrame> rst_frame;