fix: fix a bug in c apis and add more tests #24
Conversation
yangby-cryptape
commented
Mar 3, 2020
•
yangby-cryptape
commented
- refactor(tests): split tests generators
- fix(bindings/c): offsets are incorrect when all items in a Vector are None
- test: test all test vectors in C apis
- test: add proptest for both Rust and C
|
Fix the follow test cases in C: molecule/test/vectors/simple.yaml Lines 263 to 280 in a1eaca3 |
yangby-cryptape
requested review from
a team and
keroro520
and removed request for
a team and
keroro520
yangby-cryptape
requested review from
a team and
doitian
and removed request for
a team
| @@ -262,15 +262,13 @@ MOLECULE_API_DECORATOR mol_seg_res_t mol_dynvec_builder_finalize(mol_builder_t b | |||
| res.seg.size = MOL_NUM_T_SIZE + builder.number_used + builder.data_used; | |||
| res.seg.ptr = (uint8_t*)malloc(res.seg.size); | |||
| mol_pack_number(res.seg.ptr, &res.seg.size); | |||
| if (builder.data_used > 0) { | |||
There was a problem hiding this comment.
When N None were pushed into a Vector (N >= 1), the data_used is still 0, but N offsets have to be saved.
|
This is not a hard rule but next time I would suggest we split the test part and the bug fix into 2 PRs. So when I'm looking at a bug fix, I know exactly what the bug covers, while mixing 2 tasks in one PR like this makes it harder to read. |
OK, I'll pay attention next time. I found the bug when I added tests. So I put them into one PR. The tests can approve the fix works. For current PR, I split the commits for different sub-tasks, combine all sub-tasks is the main target: add more tests for C. |
|
🚢 |
