Give a specific category to each test.

[TeBoring]

This change introduce a TestCategory enum to ConformanceRequest. Existing tests
are divided into three categories: binary format test, json format test and json
format (ignore unknown when parsing) test. For the previous two categories, there
is no change to existing testee programs. For tests with the last category, testee programs
should either enable ignoring unknown field during json parsing or skip the test.

[xfxyjwf]

"Each test is given a unique test category" seems to indicate each test has a different category...

[xfxyjwf]

"spedific"?

[xfxyjwf]

Add a link to https://developers.google.com/protocol-buffers/docs/proto3#json_options and point out this is an optional feature of proto3 JSON.

[thomasvl]

@xfxyjwf did you want to also require the required/suggested concept in favor of categories?

[xfxyjwf]

@thomasvl I don't understand your question...

[thomasvl]

We also have enum ConformanceLevel which we document as as categories, should that get retired and modeled instead by the enum provided in the conformance.proto instead? They testees can also choose to say the "recommended" ones aren't supported just like they can do for these new json unknown parsing tests.

[xfxyjwf]

No, I did not indent to replace that. I think test category is different from conformance level. Test category is to group tests into feature sets. It's ok to not implement a certain optional feature so we can allow testees to skip tests for that feature entirely. However, failing recommended tests should be discouraged. Instead of making it more skippable, I would rather go the opposite direction and make it unskippable.

[thomasvl]

No, I did not indent to replace that. I think test category is different from conformance level. Test category is to group tests into feature sets. It's ok to not implement a certain optional feature so we can allow testees to skip tests for that feature entirely. However, failing recommended tests should be discouraged. Instead of making it more skippable, I would rather go the opposite direction and make it unskippable.

To that last point, a testee doesn't get them by default (I missed this at first when we supported the tests for SwiftProtobuf). You have to know to pass an extra arg when invoking the runner: https://github.com/google/protobuf/blob/master/conformance/Makefile.am#L329-L367

If you really want them by default, maybe flip the command line flag so instead of --enforce_recommended you have --skip_recommended, and then folks have to go out of their way not to run time.

[xfxyjwf]

If you really want them by default, maybe flip the command line flag so instead of --enforce_recommended you have --skip_recommended, and then folks have to go out of their way not to run time.

Yeah, I think that's what we should do. I can do that after the current conformance test cleanup is done. #4968

[TeBoring]

Is there any more change needed?

[xfxyjwf]

When a test fails, can you make sure the test category is included in the error message?

[TeBoring]

When tests fail, the error log shows something like:

json_payload: "{
  \"optionalAny\": {
     \"@type\": \"type.googleapis.com/google.protobuf.Value\",
     \"value\": 2
  }
}"
requested_output_format: JSON
message_type: "protobuf_test_messages.proto3.TestAllTypesProto3"
test_category: JSON_TEST

[xfxyjwf]

Seems good enough.