----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69600/#review211694 -----------------------------------------------------------
Looks good! Mainly I think nan, inf, and so on will be parsed, so we'll need to test that and prevent it. src/tests/resource_quantities_tests.cpp Lines 41 (patched) <https://reviews.apache.org/r/69600/#comment297186> How about FromStringValid? src/tests/resource_quantities_tests.cpp Lines 56-61 (patched) <https://reviews.apache.org/r/69600/#comment297190> s/whitespaces/whitespace This will need to be updated, probably you just want to show triming, e.g.: ``` "cpus : 1 ; mem : 1024 ; disk : 1024" ``` src/tests/resource_quantities_tests.cpp Lines 63-72 (patched) <https://reviews.apache.org/r/69600/#comment297191> Seems a little odd to test that parsing 0 is retained via the two resources case. src/tests/resource_quantities_tests.cpp Lines 98 (patched) <https://reviews.apache.org/r/69600/#comment297187> How about FromStringInvalid? src/tests/resource_quantities_tests.cpp Lines 100 (patched) <https://reviews.apache.org/r/69600/#comment297188> More like "Invalid scalar"? src/tests/resource_quantities_tests.cpp Lines 101 (patched) <https://reviews.apache.org/r/69600/#comment297185> No newline here to be consistent with the other blocks below? src/tests/resource_quantities_tests.cpp Lines 113-115 (patched) <https://reviews.apache.org/r/69600/#comment297189> Can you also add a section to test that "nan", "-nan", "inf", "-inf", "infinity", "-infinity" are disallowed? I think they will go through, so we probably have to update values::parse - Benjamin Mahler On Jan. 4, 2019, 8:14 p.m., Meng Zhu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69600/ > ----------------------------------------------------------- > > (Updated Jan. 4, 2019, 8:14 p.m.) > > > Review request for mesos and Benjamin Mahler. > > > Repository: mesos > > > Description > ------- > > Added parsing and insertion tests for `ResourceQuantities`. > > > Diffs > ----- > > src/Makefile.am 7a4904a3d67479267087fd2313a263d8218843fa > src/tests/CMakeLists.txt c588183e9d2b1cc733fdf3df70f37d47a5fdd7c0 > src/tests/resource_quantities_tests.cpp PRE-CREATION > > > Diff: https://reviews.apache.org/r/69600/diff/3/ > > > Testing > ------- > > make check > > > Thanks, > > Meng Zhu > >
