Modify DeterministicSocketData to verify that the sequence number of reads and writes start at zero…

net/socket/socket_test_util.cc

Index: net/socket/socket_test_util.cc
diff --git a/net/socket/socket_test_util.cc b/net/socket/socket_test_util.cc
index ef6d38c2e6c5c67955e5ebb45140ccfdfc32102a..1f58b421220401a43586a6cb78a1171097392c74 100644
--- a/net/socket/socket_test_util.cc
+++ b/net/socket/socket_test_util.cc
@@ -163,6 +163,22 @@ StaticSocketDataProvider::StaticSocketDataProvider(MockRead* reads,
       write_count_(writes_count) {
 }
 
+StaticSocketDataProvider::StaticSocketDataProvider(
+    bool verify_sequence_numbers,
+    MockRead* reads,
+    size_t reads_count,
+    MockWrite* writes,
+    size_t writes_count)
+    : reads_(reads),
+      read_index_(0),
+      read_count_(reads_count),
+      writes_(writes),
+      write_index_(0),
+      write_count_(writes_count) {
+  if (verify_sequence_numbers)
+    VerifyCorrectSequenceNumbers();
+}
+
 StaticSocketDataProvider::~StaticSocketDataProvider() {}
 
 const MockRead& StaticSocketDataProvider::PeekRead() const {
@@ -227,6 +243,30 @@ void StaticSocketDataProvider::Reset() {
   write_index_ = 0;
 }
 
+void StaticSocketDataProvider::VerifyCorrectSequenceNumbers() {
+  size_t read = 0;
+  size_t write = 0;
+  int expected = 0;
+  while (read < read_count_ || write < write_count_) {
+    // Check to see that we have a read or write at the expected
+    // state.
+    if (read < read_count_  && reads_[read].sequence_number == expected) {
+      ++read;
+      ++expected;
+      continue;
+    }
+    if (write < write_count_ && writes_[write].sequence_number == expected) {
+      ++write;
+      ++expected;
+      continue;
+    }
+    EXPECT_TRUE(false) << "Missing sequence number: " << expected;
+    return;
+  }
+  EXPECT_EQ(read, read_count_);
+  EXPECT_EQ(write, write_count_);

[Ryan Sleevi, 2012/07/18 18:02:13]

Rather than EXPECT_TRUE(false) [which is really ADD_FAILURE()] and EXPECT_EQ, I
think it would be better to DCHECK and NOTREACHED()

These aren't failures within the test, these are failures with the code itself.
We already DCHECK on certain situations (eg: see PeekRead/PeakWrite()), and I
think the quicker we surface to the developer that they've made a programming
error, the better.

[Ryan Hamilton, 2012/07/18 18:45:06]

Done.  Fair enough.  Using EXPECT happened to be super handy when I was working
on this feature so I could see how many tests were afflicted by this
anti-pattern, but that's not really relevant anymore.

+}
+
 DynamicSocketDataProvider::DynamicSocketDataProvider()
     : short_read_limit_(0),
       allow_unconsumed_reads_(false) {
@@ -443,7 +483,7 @@ OrderedSocketData::~OrderedSocketData() {}
 
 DeterministicSocketData::DeterministicSocketData(MockRead* reads,
     size_t reads_count, MockWrite* writes, size_t writes_count)
-    : StaticSocketDataProvider(reads, reads_count, writes, writes_count),
+    : StaticSocketDataProvider(true, reads, reads_count, writes, writes_count),
       sequence_number_(0),
       current_read_(),
       current_write_(),