gortiz commented on PR #11712:
URL: https://github.com/apache/pinot/pull/11712#issuecomment-1742784997
I've changed the code to:
- Recover the logic in master (use isOptional)
- Add more tests
- Renamed `ProtoBufRecordExtractorSimpleTest` as
`ProtoBufRecordExtractorProgrammaticTest`
- Add `ProtoBufRecordExtractorLowLevelTest` which runs tests using the
same protocol definition used by `ProtoBufRecordExtractorTest` but doesn't
include all the error prone magic in `AbstractRecordExtractorTest`.
I think `ProtoBufRecordExtractorLowLevelTest` is easier to read. There are 4
test cases:
1. When the field is set to some non default value
2. When the field is set to the default value
3. When the field is not set
4. When the field is clear (which should be semantically equal to 3, but I
wanted to be sure.
It is easier and more useful to read the DataProviders than the tests.
As you can see, both `ProtoBufRecordExtractorProgrammaticTest` and
`ProtoBufRecordExtractorLowLevelTest` have some test that fail. All these fails
can be fixed by recovering Shen's code where we used `isOptionalKeyword`
instead of `isOptional`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
