diff --git a/conformance/Makefile.am b/conformance/Makefile.am index 95343f140b..cccbac9e68 100644 --- a/conformance/Makefile.am +++ b/conformance/Makefile.am @@ -57,7 +57,7 @@ conformance-java: javac_middleman # Targets for actually running tests. test_cpp: protoc_middleman conformance-test-runner conformance-cpp - ./conformance-test-runner ./conformance-cpp + ./conformance-test-runner --failure_list failure_list_cpp.txt ./conformance-cpp test_java: protoc_middleman conformance-test-runner conformance-java ./conformance-test-runner ./conformance-java diff --git a/conformance/conformance.proto b/conformance/conformance.proto index 39eafdbb7c..892db380c7 100644 --- a/conformance/conformance.proto +++ b/conformance/conformance.proto @@ -57,11 +57,13 @@ 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 = 1; - string json_payload = 2; + bytes protobuf_payload = 2; + string json_payload = 3; } enum RequestedOutput { @@ -71,7 +73,7 @@ message ConformanceRequest { } // Which format should the testee serialize its message to? - RequestedOutput requested_output = 3; + RequestedOutput requested_output = 4; } // Represents a single test case's output. diff --git a/conformance/conformance_test.cc b/conformance/conformance_test.cc index 857f215296..d441137dea 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 diff --git a/conformance/conformance_test.h b/conformance/conformance_test.h index c16f9c0bb9..651088f758 100644 --- a/conformance/conformance_test.h +++ b/conformance/conformance_test.h @@ -83,24 +83,49 @@ class ConformanceTestSuite { public: ConformanceTestSuite() : verbose_(false) {} + // Sets the list of tests that are expected to fail when RunSuite() is called. + // RunSuite() will fail unless the set of failing tests is exactly the same + // as this list. + void SetFailureList(const std::vector& failure_list); + // Run all the conformance tests against the given test runner. // Test output will be stored in "output". - void RunSuite(ConformanceTestRunner* runner, std::string* output); + // + // Returns true if the set of failing tests was exactly the same as the + // failure list. If SetFailureList() was not called, returns true if all + // tests passed. + bool RunSuite(ConformanceTestRunner* runner, std::string* output); private: - void ReportSuccess(); - void ReportFailure(const char* fmt, ...); + void ReportSuccess(const std::string& test_name); + void ReportFailure(const std::string& test_name, const char* fmt, ...); void RunTest(const conformance::ConformanceRequest& request, conformance::ConformanceResponse* response); - void DoExpectParseFailureForProto(const std::string& proto, int line); - void TestPrematureEOFForType( - google::protobuf::internal::WireFormatLite::FieldType type); - + void ExpectParseFailureForProto(const std::string& proto, + const std::string& test_name); + void ExpectHardParseFailureForProto(const std::string& proto, + const std::string& test_name); + void TestPrematureEOFForType(google::protobuf::FieldDescriptor::Type type); + bool CheckSetEmpty(const set& set_to_check, const char* msg); ConformanceTestRunner* runner_; int successes_; int failures_; bool verbose_; std::string output_; + + // The set of test names that are expected to fail in this run, but haven't + // failed yet. + std::set expected_to_fail_; + + // The set of test names that have been run. Used to ensure that there are no + // duplicate names in the suite. + std::set test_names_; + + // The set of tests that failed, but weren't expected to. + std::set unexpected_failing_tests_; + + // The set of tests that succeeded, but weren't expected to. + std::set unexpected_succeeding_tests_; }; } // namespace protobuf diff --git a/conformance/conformance_test_runner.cc b/conformance/conformance_test_runner.cc index e0bb50a5e5..b56e19cfbe 100644 --- a/conformance/conformance_test_runner.cc +++ b/conformance/conformance_test_runner.cc @@ -55,6 +55,8 @@ #include #include +#include +#include #include "conformance.pb.h" #include "conformance_test.h" @@ -62,6 +64,8 @@ using conformance::ConformanceRequest; using conformance::ConformanceResponse; using google::protobuf::internal::scoped_array; +using std::string; +using std::vector; #define STRINGIFY(x) #x #define TOSTRING(x) STRINGIFY(x) @@ -180,18 +184,67 @@ class ForkPipeRunner : public google::protobuf::ConformanceTestRunner { std::string executable_; }; +void UsageError() { + fprintf(stderr, + "Usage: conformance-test-runner [options] \n"); + fprintf(stderr, "\n"); + fprintf(stderr, "Options:\n"); + fprintf(stderr, + " --failure_list Use to specify list of tests\n"); + fprintf(stderr, + " that are expected to fail. File\n"); + fprintf(stderr, + " should contain one test name per\n"); + fprintf(stderr, + " line. Use '#' for comments.\n"); + exit(1); +} + +void ParseFailureList(const char *filename, vector* failure_list) { + std::ifstream infile(filename); + for (string line; getline(infile, line);) { + // Remove whitespace. + line.erase(std::remove_if(line.begin(), line.end(), ::isspace), + line.end()); + + // Remove comments. + line = line.substr(0, line.find("#")); + + if (!line.empty()) { + failure_list->push_back(line); + } + } +} int main(int argc, char *argv[]) { - if (argc < 2) { - fprintf(stderr, "Usage: conformance-test-runner \n"); - exit(1); + int arg = 1; + char *program; + vector failure_list; + + for (int arg = 1; arg < argc; ++arg) { + if (strcmp(argv[arg], "--failure_list") == 0) { + if (++arg == argc) UsageError(); + ParseFailureList(argv[arg], &failure_list); + } else if (argv[arg][0] == '-') { + fprintf(stderr, "Unknown option: %s\n", argv[arg]); + UsageError(); + } else { + if (arg != argc - 1) { + fprintf(stderr, "Too many arguments.\n"); + UsageError(); + } + program = argv[arg]; + } } - ForkPipeRunner runner(argv[1]); + ForkPipeRunner runner(program); google::protobuf::ConformanceTestSuite suite; + suite.SetFailureList(failure_list); std::string output; - suite.RunSuite(&runner, &output); + bool ok = suite.RunSuite(&runner, &output); fwrite(output.c_str(), 1, output.size(), stderr); + + return ok ? EXIT_SUCCESS : EXIT_FAILURE; } diff --git a/conformance/failure_list_cpp.txt b/conformance/failure_list_cpp.txt new file mode 100644 index 0000000000..cda1d4a14e --- /dev/null +++ b/conformance/failure_list_cpp.txt @@ -0,0 +1,21 @@ +# This is the list of conformance tests that are known to fail for the C++ +# implementation right now. These should be fixed. +# +# By listing them here we can keep tabs on which ones are failing and be sure +# 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. + +PrematureEofBeforeKnownRepeatedValue.MESSAGE +PrematureEofInDelimitedDataForKnownNonRepeatedValue.MESSAGE +PrematureEofInDelimitedDataForKnownRepeatedValue.MESSAGE +PrematureEofInPackedField.BOOL +PrematureEofInPackedField.ENUM +PrematureEofInPackedField.INT32 +PrematureEofInPackedField.INT64 +PrematureEofInPackedField.SINT32 +PrematureEofInPackedField.SINT64 +PrematureEofInPackedField.UINT32 +PrematureEofInPackedField.UINT64 +PrematureEofInsideKnownRepeatedValue.MESSAGE