----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72840/#review221837 -----------------------------------------------------------
Fix it, then Ship it! src/tests/master/update_framework_tests.cpp Lines 914 (patched) <https://reviews.apache.org/r/72840/#comment310918> Remove this? and consider putting it on the assertion instead? E.g. ``` ASSERT_SOME_EQ(JSON::protobuf(updatedConstraints), reportedConstraints) << "Expected: " << ... "\n vs actual: " << ...; ``` Is this not printing out the two correctly upon failure? src/tests/master/update_framework_tests.cpp Lines 919 (patched) <https://reviews.apache.org/r/72840/#comment310920> It's a little hard to see what it's supposed to look like based on this, seems preferrable to me to check the expected json in some fashion? E.g. ``` JSON::Value expected = ... "offer_constraints: { ... }" ``` Like we do in other tests, that way, you can read the test and know what the API response is supposed to look like? src/tests/master/update_framework_tests.cpp Lines 921 (patched) <https://reviews.apache.org/r/72840/#comment310919> Any reason not to just implement the check? - Benjamin Mahler On Sept. 7, 2020, 3:27 p.m., Andrei Sekretenko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/72840/ > ----------------------------------------------------------- > > (Updated Sept. 7, 2020, 3:27 p.m.) > > > Review request for mesos and Benjamin Mahler. > > > Bugs: MESOS-10179 > https://issues.apache.org/jira/browse/MESOS-10179 > > > Repository: mesos > > > Description > ------- > > Exposed offer constraints via the `/state` and `/frameworks` endpoints. > > > Diffs > ----- > > src/master/readonly_handler.cpp b1336f9aa849e826a78c3fe4bb83e3efeb186a31 > src/tests/master/update_framework_tests.cpp > 87bc3c7d23ea76118473d9a4ec3c77e7a9d5a97e > > > Diff: https://reviews.apache.org/r/72840/diff/1/ > > > Testing > ------- > > > Thanks, > > Andrei Sekretenko > >
