diff --git a/dvaas/test_run_validation.cc b/dvaas/test_run_validation.cc index 55791394..a56089f0 100644 --- a/dvaas/test_run_validation.cc +++ b/dvaas/test_run_validation.cc @@ -99,11 +99,13 @@ bool CompareSwitchOutputs(SwitchOutput actual_output, return false; } - if (actual_output.packet_ins_size() != expected_output.packet_ins_size()) { - *listener << "has mismatched number of packet ins (actual: " - << actual_output.packet_ins_size() - << " expected: " << expected_output.packet_ins_size() << ")\n"; - return false; + if (!params.treat_expected_and_actual_outputs_as_having_no_packet_ins) { + if (actual_output.packet_ins_size() != expected_output.packet_ins_size()) { + *listener << "has mismatched number of packet ins (actual: " + << actual_output.packet_ins_size() + << " expected: " << expected_output.packet_ins_size() << ")\n"; + return false; + } } std::sort(actual_output.mutable_packets()->pointer_begin(), @@ -139,58 +141,63 @@ bool CompareSwitchOutputs(SwitchOutput actual_output, } } - for (int i = 0; i < expected_output.packet_ins_size(); ++i) { - const PacketIn& actual_packet_in = actual_output.packet_ins(i); - const PacketIn& expected_packet_in = expected_output.packet_ins(i); - - MessageDifferencer differ; - for (auto* field : params.ignored_packetlib_fields) - differ.IgnoreField(field); - std::string diff; - differ.ReportDifferencesToString(&diff); - if (!differ.Compare(expected_packet_in.parsed(), - actual_packet_in.parsed())) { - *listener << "has packet in " << i - << " with mismatched header fields:\n " << Indent(2, diff); - return false; - } - - // Check that received packet in has desired metadata (except for ignored - // metadata). - for (const auto& expected_metadata : expected_packet_in.metadata()) { - if (params.ignored_packet_in_metadata.contains(expected_metadata.name())) - continue; - - std::optional actual_metadata = - GetPacketInMetadataByName(actual_packet_in, expected_metadata.name()); - if (!actual_metadata.has_value()) { - *listener << "has packet in " << i << " with missing metadata field '" - << expected_metadata.name() << "':\n " << Indent(2, diff); - return false; - } - - if (!differ.Compare(expected_metadata.value(), - actual_metadata->value())) { + if (!params.treat_expected_and_actual_outputs_as_having_no_packet_ins) { + for (int i = 0; i < expected_output.packet_ins_size(); ++i) { + const PacketIn& actual_packet_in = actual_output.packet_ins(i); + const PacketIn& expected_packet_in = expected_output.packet_ins(i); + + MessageDifferencer differ; + for (auto* field : params.ignored_packetlib_fields) + differ.IgnoreField(field); + std::string diff; + differ.ReportDifferencesToString(&diff); + if (!differ.Compare(expected_packet_in.parsed(), + actual_packet_in.parsed())) { *listener << "has packet in " << i - << " with mismatched value for metadata field '" - << expected_metadata.name() << "':\n " << Indent(2, diff); + << " with mismatched header fields:\n " << Indent(2, diff); return false; } - } - // Check that received packet in does not have additional metadata (except - // for ignored metadata). - for (const auto& actual_metadata : actual_packet_in.metadata()) { - if (params.ignored_packet_in_metadata.contains(actual_metadata.name())) - continue; + // Check that received packet in has desired metadata (except for ignored + // metadata). + for (const auto& expected_metadata : expected_packet_in.metadata()) { + if (params.ignored_packet_in_metadata.contains( + expected_metadata.name())) + continue; + + std::optional actual_metadata = + GetPacketInMetadataByName(actual_packet_in, + expected_metadata.name()); + if (!actual_metadata.has_value()) { + *listener << "has packet in " << i << " with missing metadata field '" + << expected_metadata.name() << "':\n " << Indent(2, diff); + return false; + } + + if (!differ.Compare(expected_metadata.value(), + actual_metadata->value())) { + *listener << "has packet in " << i + << " with mismatched value for metadata field '" + << expected_metadata.name() << "':\n " << Indent(2, diff); + return false; + } + } - std::optional expected_metadata = - GetPacketInMetadataByName(expected_packet_in, actual_metadata.name()); - if (!expected_metadata.has_value()) { - *listener << "has packet in " << i - << " with additional metadata field '" - << actual_metadata.name() << "':\n " << Indent(2, diff); - return false; + // Check that received packet in does not have additional metadata (except + // for ignored metadata). + for (const auto& actual_metadata : actual_packet_in.metadata()) { + if (params.ignored_packet_in_metadata.contains(actual_metadata.name())) + continue; + + std::optional expected_metadata = + GetPacketInMetadataByName(expected_packet_in, + actual_metadata.name()); + if (!expected_metadata.has_value()) { + *listener << "has packet in " << i + << " with additional metadata field '" + << actual_metadata.name() << "':\n " << Indent(2, diff); + return false; + } } } } @@ -370,6 +377,9 @@ PacketTestValidationResult ValidateTestRun( }), ")"); } + if (diff_params.treat_expected_and_actual_outputs_as_having_no_packet_ins) { + absl::StrAppend(&expectation, "\n (ignoring packet-ins)"); + } std::string actual = DescribeActual(test_run.test_vector().input(), actual_output_characterization); if (actual_matches_expected) { diff --git a/dvaas/test_run_validation.h b/dvaas/test_run_validation.h index 1d78f43d..f85894fb 100644 --- a/dvaas/test_run_validation.h +++ b/dvaas/test_run_validation.h @@ -43,6 +43,14 @@ struct SwitchOutputDiffParams { // in `ignored_metadata_for_validation`, the field is ignored during // comparison. absl::flat_hash_set ignored_packet_in_metadata; + + // If true, packet-ins (punted packets) are ignored while comparing expected + // and actual output. Note that packet-ins are completely ignored on both + // sides. Effectively the expected and actual outputs are compared assuming + // neither has any packet-ins. + // TODO: Remove and replace with + // `ignore_missing_packet_ins_in_actual_output`. + bool treat_expected_and_actual_outputs_as_having_no_packet_ins = false; }; // Validates the given `test_vector` using the parameters in `params` and diff --git a/dvaas/test_run_validation_test.cc b/dvaas/test_run_validation_test.cc index 0a166bf0..9b0c9d66 100644 --- a/dvaas/test_run_validation_test.cc +++ b/dvaas/test_run_validation_test.cc @@ -132,5 +132,106 @@ TEST(TestRunValidationTest, HasSubstr("mismatched ports:")); } +TEST(TestRunValidationTest, + MissingPacketInsAreIgnoredIfAndOnlyIfIgnorePacketInsIsSet) { + const PacketTestRun test_run = gutil::ParseProtoOrDie(R"pb( + test_vector { + acceptable_outputs { + packets { port: "1" } + packet_ins { + metadata { + name: "target_egress_port" + value { str: "1" } + } + } + } + } + actual_output { packets { port: "1" } } + )pb"); + + // Without ignoring packet-ins, validation must fail. + ASSERT_THAT(ValidateTestRun(test_run).failure().description(), + HasSubstr("packet in")); + + // Ignoring packet-ins, validation must succeed. + ASSERT_THAT( + ValidateTestRun( + test_run, + { + .treat_expected_and_actual_outputs_as_having_no_packet_ins = true, + }), + EqualsProto(R"pb()pb")); +} + +TEST(TestRunValidationTest, + PacketInDifferencesAreIgnoredIfAndOnlyIfIgnorePacketInsIsSet) { + const PacketTestRun test_run = gutil::ParseProtoOrDie(R"pb( + test_vector { + acceptable_outputs { + packets { port: "1" } + packet_ins { + metadata { + name: "target_egress_port" + value { str: "1" } + } + } + } + } + actual_output { + packets { port: "1" } + packet_ins { + metadata { + name: "target_egress_port" + value { str: "2" } + } + } + } + )pb"); + + // Without ignoring packet-ins, validation must fail. + ASSERT_THAT(ValidateTestRun(test_run).failure().description(), + HasSubstr("target_egress_port")); + + // Ignoring packet-ins, validation must succeed. + ASSERT_THAT( + ValidateTestRun( + test_run, + { + .treat_expected_and_actual_outputs_as_having_no_packet_ins = true, + }), + EqualsProto(R"pb()pb")); +} + +TEST(TestRunValidationTest, IgnorePacketInsHasNoEffectWhenPacketInsMatch) { + ASSERT_THAT( + ValidateTestRun( + gutil::ParseProtoOrDie(R"pb( + test_vector { + acceptable_outputs { + packets { port: "1" } + packet_ins { + metadata { + name: "target_egress_port" + value { str: "1" } + } + } + } + } + actual_output { + packets { port: "1" } + packet_ins { + metadata { + name: "target_egress_port" + value { str: "1" } + } + } + } + )pb"), + { + .treat_expected_and_actual_outputs_as_having_no_packet_ins = true, + }), + EqualsProto(R"pb()pb")); +} + } // namespace } // namespace dvaas