From d2b67389b30d1bff1080be3a3d8d7cfb0b81b867 Mon Sep 17 00:00:00 2001 From: Josh Haberman Date: Wed, 3 Jun 2015 12:04:35 -0700 Subject: Conformance tests can now be excluded based on their names. This allows us to enable conformance tests even when we know that some tests are failing and need to be fixed. Change-Id: I372f43663008747db6f2b2cf06e6ffa4c6d85b2d --- conformance/conformance_test.cc | 160 +++++++++++++++++++++++++++++----------- 1 file changed, 117 insertions(+), 43 deletions(-) (limited to 'conformance/conformance_test.cc') diff --git a/conformance/conformance_test.cc b/conformance/conformance_test.cc index 857f2152..d441137d 100644 --- a/conformance/conformance_test.cc +++ b/conformance/conformance_test.cc @@ -126,12 +126,11 @@ string submsg(uint32_t fn, const string& buf) { #define UNKNOWN_FIELD 666 -uint32_t GetFieldNumberForType(WireFormatLite::FieldType type, bool repeated) { +uint32_t GetFieldNumberForType(FieldDescriptor::Type type, bool repeated) { const Descriptor* d = TestAllTypes().GetDescriptor(); for (int i = 0; i < d->field_count(); i++) { const FieldDescriptor* f = d->field(i); - if (static_cast(f->type()) == type && - f->is_repeated() == repeated) { + if (f->type() == type && f->is_repeated() == repeated) { return f->number(); } } @@ -139,16 +138,37 @@ uint32_t GetFieldNumberForType(WireFormatLite::FieldType type, bool repeated) { return 0; } +string UpperCase(string str) { + for (int i = 0; i < str.size(); i++) { + str[i] = toupper(str[i]); + } + return str; +} + } // anonymous namespace namespace google { namespace protobuf { -void ConformanceTestSuite::ReportSuccess() { +void ConformanceTestSuite::ReportSuccess(const string& test_name) { + if (expected_to_fail_.erase(test_name) != 0) { + StringAppendF(&output_, + "ERROR: test %s is in the failure list, but test succeeded. " + "Remove it from the failure list.\n", + test_name.c_str()); + unexpected_succeeding_tests_.insert(test_name); + } successes_++; } -void ConformanceTestSuite::ReportFailure(const char *fmt, ...) { +void ConformanceTestSuite::ReportFailure(const string& test_name, + const char* fmt, ...) { + if (expected_to_fail_.erase(test_name) == 1) { + StringAppendF(&output_, "FAILED AS EXPECTED: "); + } else { + StringAppendF(&output_, "ERROR: "); + unexpected_failing_tests_.insert(test_name); + } va_list args; va_start(args, fmt); StringAppendV(&output_, fmt, args); @@ -158,6 +178,10 @@ void ConformanceTestSuite::ReportFailure(const char *fmt, ...) { void ConformanceTestSuite::RunTest(const ConformanceRequest& request, ConformanceResponse* response) { + if (test_names_.insert(request.test_name()).second == false) { + GOOGLE_LOG(FATAL) << "Duplicated test name: " << request.test_name(); + } + string serialized_request; string serialized_response; request.SerializeToString(&serialized_request); @@ -176,10 +200,12 @@ void ConformanceTestSuite::RunTest(const ConformanceRequest& request, } } -void ConformanceTestSuite::DoExpectParseFailureForProto(const string& proto, - int line) { +// Expect that this precise protobuf will cause a parse error. +void ConformanceTestSuite::ExpectParseFailureForProto( + const string& proto, const string& test_name) { ConformanceRequest request; ConformanceResponse response; + request.set_test_name(test_name); request.set_protobuf_payload(proto); // We don't expect output, but if the program erroneously accepts the protobuf @@ -188,29 +214,27 @@ void ConformanceTestSuite::DoExpectParseFailureForProto(const string& proto, RunTest(request, &response); if (response.result_case() == ConformanceResponse::kParseError) { - ReportSuccess(); + ReportSuccess(test_name); } else { - ReportFailure("Should have failed, but didn't. Line: %d, Request: %s, " + ReportFailure(test_name, + "Should have failed to parse, but didn't. Request: %s, " "response: %s\n", - line, request.ShortDebugString().c_str(), response.ShortDebugString().c_str()); } } -// Expect that this precise protobuf will cause a parse error. -#define ExpectParseFailureForProto(proto) DoExpectParseFailureForProto(proto, __LINE__) - // Expect that this protobuf will cause a parse error, even if it is followed // by valid protobuf data. We can try running this twice: once with this // data verbatim and once with this data followed by some valid data. // // TODO(haberman): implement the second of these. -#define ExpectHardParseFailureForProto(proto) DoExpectParseFailureForProto(proto, __LINE__) - +void ConformanceTestSuite::ExpectHardParseFailureForProto( + const string& proto, const string& test_name) { + return ExpectParseFailureForProto(proto, test_name); +} -void ConformanceTestSuite::TestPrematureEOFForType( - WireFormatLite::FieldType type) { +void ConformanceTestSuite::TestPrematureEOFForType(FieldDescriptor::Type type) { // Incomplete values for each wire type. static const string incompletes[6] = { string("\x80"), // VARINT @@ -223,45 +247,51 @@ void ConformanceTestSuite::TestPrematureEOFForType( uint32_t fieldnum = GetFieldNumberForType(type, false); uint32_t rep_fieldnum = GetFieldNumberForType(type, true); - WireFormatLite::WireType wire_type = - WireFormatLite::WireTypeForFieldType(type); + WireFormatLite::WireType wire_type = WireFormatLite::WireTypeForFieldType( + static_cast(type)); const string& incomplete = incompletes[wire_type]; + const string type_name = + UpperCase(string(".") + FieldDescriptor::TypeName(type)); - // EOF before a known non-repeated value. - ExpectParseFailureForProto(tag(fieldnum, wire_type)); + ExpectParseFailureForProto( + tag(fieldnum, wire_type), + "PrematureEofBeforeKnownNonRepeatedValue" + type_name); - // EOF before a known repeated value. - ExpectParseFailureForProto(tag(rep_fieldnum, wire_type)); + ExpectParseFailureForProto( + tag(rep_fieldnum, wire_type), + "PrematureEofBeforeKnownRepeatedValue" + type_name); - // EOF before an unknown value. - ExpectParseFailureForProto(tag(UNKNOWN_FIELD, wire_type)); + ExpectParseFailureForProto( + tag(UNKNOWN_FIELD, wire_type), + "PrematureEofBeforeUnknownValue" + type_name); - // EOF inside a known non-repeated value. ExpectParseFailureForProto( - cat( tag(fieldnum, wire_type), incomplete )); + cat( tag(fieldnum, wire_type), incomplete ), + "PrematureEofInsideKnownNonRepeatedValue" + type_name); - // EOF inside a known repeated value. ExpectParseFailureForProto( - cat( tag(rep_fieldnum, wire_type), incomplete )); + cat( tag(rep_fieldnum, wire_type), incomplete ), + "PrematureEofInsideKnownRepeatedValue" + type_name); - // EOF inside an unknown value. ExpectParseFailureForProto( - cat( tag(UNKNOWN_FIELD, wire_type), incomplete )); + cat( tag(UNKNOWN_FIELD, wire_type), incomplete ), + "PrematureEofInsideUnknownValue" + type_name); if (wire_type == WireFormatLite::WIRETYPE_LENGTH_DELIMITED) { - // EOF in the middle of delimited data for known non-repeated value. ExpectParseFailureForProto( - cat( tag(fieldnum, wire_type), varint(1) )); + cat( tag(fieldnum, wire_type), varint(1) ), + "PrematureEofInDelimitedDataForKnownNonRepeatedValue" + type_name); - // EOF in the middle of delimited data for known repeated value. ExpectParseFailureForProto( - cat( tag(rep_fieldnum, wire_type), varint(1) )); + cat( tag(rep_fieldnum, wire_type), varint(1) ), + "PrematureEofInDelimitedDataForKnownRepeatedValue" + type_name); // EOF in the middle of delimited data for unknown value. ExpectParseFailureForProto( - cat( tag(UNKNOWN_FIELD, wire_type), varint(1) )); + cat( tag(UNKNOWN_FIELD, wire_type), varint(1) ), + "PrematureEofInDelimitedDataForUnknownValue" + type_name); - if (type == WireFormatLite::TYPE_MESSAGE) { + if (type == FieldDescriptor::TYPE_MESSAGE) { // Submessage ends in the middle of a value. string incomplete_submsg = cat( tag(WireFormatLite::TYPE_INT32, WireFormatLite::WIRETYPE_VARINT), @@ -269,42 +299,86 @@ void ConformanceTestSuite::TestPrematureEOFForType( ExpectHardParseFailureForProto( cat( tag(fieldnum, WireFormatLite::WIRETYPE_LENGTH_DELIMITED), varint(incomplete_submsg.size()), - incomplete_submsg )); + incomplete_submsg ), + "PrematureEofInSubmessageValue" + type_name); } - } else if (type != WireFormatLite::TYPE_GROUP) { + } else if (type != FieldDescriptor::TYPE_GROUP) { // Non-delimited, non-group: eligible for packing. // Packed region ends in the middle of a value. ExpectHardParseFailureForProto( cat( tag(rep_fieldnum, WireFormatLite::WIRETYPE_LENGTH_DELIMITED), varint(incomplete.size()), - incomplete )); + incomplete ), + "PrematureEofInPackedFieldValue" + type_name); // EOF in the middle of packed region. ExpectParseFailureForProto( cat( tag(rep_fieldnum, WireFormatLite::WIRETYPE_LENGTH_DELIMITED), - varint(1) )); + varint(1) ), + "PrematureEofInPackedField" + type_name); } } -void ConformanceTestSuite::RunSuite(ConformanceTestRunner* runner, +void ConformanceTestSuite::SetFailureList(const vector& failure_list) { + expected_to_fail_.clear(); + std::copy(failure_list.begin(), failure_list.end(), + std::inserter(expected_to_fail_, expected_to_fail_.end())); +} + +bool ConformanceTestSuite::CheckSetEmpty(const set& set_to_check, + const char* msg) { + if (set_to_check.empty()) { + return true; + } else { + StringAppendF(&output_, "\n"); + StringAppendF(&output_, "ERROR: %s:\n", msg); + for (set::const_iterator iter = set_to_check.begin(); + iter != set_to_check.end(); ++iter) { + StringAppendF(&output_, "%s\n", iter->c_str()); + } + return false; + } +} + +bool ConformanceTestSuite::RunSuite(ConformanceTestRunner* runner, std::string* output) { runner_ = runner; output_.clear(); successes_ = 0; failures_ = 0; + test_names_.clear(); + unexpected_failing_tests_.clear(); + unexpected_succeeding_tests_.clear(); for (int i = 1; i <= FieldDescriptor::MAX_TYPE; i++) { if (i == FieldDescriptor::TYPE_GROUP) continue; - TestPrematureEOFForType(static_cast(i)); + TestPrematureEOFForType(static_cast(i)); } + StringAppendF(&output_, "\n"); StringAppendF(&output_, "CONFORMANCE SUITE FINISHED: completed %d tests, %d successes, " "%d failures.\n", successes_ + failures_, successes_, failures_); + bool ok = + CheckSetEmpty(expected_to_fail_, + "These tests were listed in the failure list, but they " + "don't exist. Remove them from the failure list") && + + CheckSetEmpty(unexpected_failing_tests_, + "These tests failed. If they can't be fixed right now, " + "you can add them to the failure list so the overall " + "suite can succeed") && + + CheckSetEmpty(unexpected_succeeding_tests_, + "These tests succeeded, even though they were listed in " + "the failure list. Remove them from the failure list"); + output->assign(output_); + + return ok; } } // namespace protobuf -- cgit v1.2.3 From 23bf3b566f4bd1c5d73a5d093e2d0a45ee81ff27 Mon Sep 17 00:00:00 2001 From: Josh Haberman Date: Thu, 4 Jun 2015 15:04:00 -0700 Subject: Removed test_name from conformance.proto. Change-Id: I382dcda97fa123a6da4ff5faad5d7ece95853f33 --- conformance/conformance.proto | 8 +++----- conformance/conformance_test.cc | 17 +++++++++-------- conformance/conformance_test.h | 3 ++- conformance/failure_list_cpp.txt | 2 +- 4 files changed, 15 insertions(+), 15 deletions(-) (limited to 'conformance/conformance_test.cc') diff --git a/conformance/conformance.proto b/conformance/conformance.proto index 892db380..39eafdbb 100644 --- a/conformance/conformance.proto +++ b/conformance/conformance.proto @@ -57,13 +57,11 @@ option java_package = "com.google.protobuf.conformance"; // 2. parse the protobuf or JSON payload in "payload" (which may fail) // 3. if the parse succeeded, serialize the message in the requested format. message ConformanceRequest { - string test_name = 1; - // The payload (whether protobuf of JSON) is always for a TestAllTypes proto // (see below). oneof payload { - bytes protobuf_payload = 2; - string json_payload = 3; + bytes protobuf_payload = 1; + string json_payload = 2; } enum RequestedOutput { @@ -73,7 +71,7 @@ message ConformanceRequest { } // Which format should the testee serialize its message to? - RequestedOutput requested_output = 4; + RequestedOutput requested_output = 3; } // Represents a single test case's output. diff --git a/conformance/conformance_test.cc b/conformance/conformance_test.cc index d441137d..7a7fc6f5 100644 --- a/conformance/conformance_test.cc +++ b/conformance/conformance_test.cc @@ -164,9 +164,9 @@ void ConformanceTestSuite::ReportSuccess(const string& test_name) { void ConformanceTestSuite::ReportFailure(const string& test_name, const char* fmt, ...) { if (expected_to_fail_.erase(test_name) == 1) { - StringAppendF(&output_, "FAILED AS EXPECTED: "); + StringAppendF(&output_, "FAILED AS EXPECTED, test=%s: ", test_name.c_str()); } else { - StringAppendF(&output_, "ERROR: "); + StringAppendF(&output_, "ERROR, test=%s: ", test_name.c_str()); unexpected_failing_tests_.insert(test_name); } va_list args; @@ -176,10 +176,11 @@ void ConformanceTestSuite::ReportFailure(const string& test_name, failures_++; } -void ConformanceTestSuite::RunTest(const ConformanceRequest& request, +void ConformanceTestSuite::RunTest(const string& test_name, + const ConformanceRequest& request, ConformanceResponse* response) { - if (test_names_.insert(request.test_name()).second == false) { - GOOGLE_LOG(FATAL) << "Duplicated test name: " << request.test_name(); + if (test_names_.insert(test_name).second == false) { + GOOGLE_LOG(FATAL) << "Duplicated test name: " << test_name; } string serialized_request; @@ -194,7 +195,8 @@ void ConformanceTestSuite::RunTest(const ConformanceRequest& request, } if (verbose_) { - StringAppendF(&output_, "conformance test: request=%s, response=%s\n", + StringAppendF(&output_, "conformance test: name=%s, request=%s, response=%s\n", + test_name.c_str(), request.ShortDebugString().c_str(), response->ShortDebugString().c_str()); } @@ -205,14 +207,13 @@ void ConformanceTestSuite::ExpectParseFailureForProto( const string& proto, const string& test_name) { ConformanceRequest request; ConformanceResponse response; - request.set_test_name(test_name); request.set_protobuf_payload(proto); // We don't expect output, but if the program erroneously accepts the protobuf // we let it send its response as this. We must not leave it unspecified. request.set_requested_output(ConformanceRequest::PROTOBUF); - RunTest(request, &response); + RunTest(test_name, request, &response); if (response.result_case() == ConformanceResponse::kParseError) { ReportSuccess(test_name); } else { diff --git a/conformance/conformance_test.h b/conformance/conformance_test.h index 651088f7..764a8d33 100644 --- a/conformance/conformance_test.h +++ b/conformance/conformance_test.h @@ -99,7 +99,8 @@ class ConformanceTestSuite { private: void ReportSuccess(const std::string& test_name); void ReportFailure(const std::string& test_name, const char* fmt, ...); - void RunTest(const conformance::ConformanceRequest& request, + void RunTest(const std::string& test_name, + const conformance::ConformanceRequest& request, conformance::ConformanceResponse* response); void ExpectParseFailureForProto(const std::string& proto, const std::string& test_name); diff --git a/conformance/failure_list_cpp.txt b/conformance/failure_list_cpp.txt index cda1d4a1..d792eddd 100644 --- a/conformance/failure_list_cpp.txt +++ b/conformance/failure_list_cpp.txt @@ -5,7 +5,7 @@ # that we don't introduce regressions in other tests. # # TODO(haberman): insert links to corresponding bugs tracking the issue. -# Should we use GitHub issues or the Google-internal bug tracker. +# Should we use GitHub issues or the Google-internal bug tracker? PrematureEofBeforeKnownRepeatedValue.MESSAGE PrematureEofInDelimitedDataForKnownNonRepeatedValue.MESSAGE -- cgit v1.2.3