KF-25 add tests for encoding#36
Conversation
| uint8_t a3[3]; | ||
| uint8_t a4[4]; | ||
|
|
||
| if (output.size() < decodeLen(input)) |
| } | ||
|
|
||
| GIVEN("Empty buffer") | ||
| GIVEN("Empty buffer small buffer (<kMaximumBomLength)") |
There was a problem hiding this comment.
Empty buffer, (comma) small buffer...?
| } | ||
| } | ||
|
|
||
| GIVEN("Empty buffer big buffer (>kMaximumBomLength)") |
There was a problem hiding this comment.
Renamed to 'Empty big buffer', because there is a different behavoir for buffer bigger than kMaximumBomLength and smaller
There was a problem hiding this comment.
The word empty has conflict with big. If empty means that it is filled with zeroes, then it is just not right word for it.
|
|
||
| THEN("Encoding is set to ANSI") | ||
| { | ||
| REQUIRE(detector.getEncoding() == kf::EncodingDetector::ANSI); |
There was a problem hiding this comment.
ANSI that contains nulls? It looks like Unknown
There was a problem hiding this comment.
ANSI is used as fallback type if no BOM and no UTF-16-like null byte pattern was detected.
So I guess we should add some ANSI pattern check
There was a problem hiding this comment.
Sure, and as for me, ANSI should not be used as a fallback, Unknown is the right value for it.
There was a problem hiding this comment.
So, should I change it in this PR?
It doesn't seem related to the current task. Also, an ANSI check will need to be added because simply replacing it with Unknown will break the function
There was a problem hiding this comment.
@SergiusTheBest what do you think about my proposal and the impact?
There was a problem hiding this comment.
@SergiusTheBest what do you think about my proposal and the impact?
@belyshevdenis Yes, it makes sense. But let's create an issue for it and a separate PR with the fix.
@Kellesi I searched for usage of that function and we can change the defaults to Unknown without breaking things.
There was a problem hiding this comment.
Changed to Unknown in PR #43
Also removed test for ANSI to prevent test fail, until such check would be implemented
|
@Kellesi @belyshevdenis Guys, let's do small PRs, so we can merge code quickly, have less discussions and follow the principle "If you want to eat an elephant, do it with a teaspoon". Here we should have 4 PRs instead of 1. Actually it will make everyone happier as things will move instead of stalling for days. |
@Kellesi please split it to 4 PRs: 1 per class test, and remove changes from UString, they will be added in another PR |
Base64: |
|
Closing it as we have separate PRs now. |
Task: https://jira.dev.local/jira/browse/KF-25
Added tests for Hex, Base64, EncodingDetector, TextDetector
Added comments for tested classes