----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41384/#review110373 -----------------------------------------------------------
Ship it! I like this approach a lot. Maybe some more comments could be useful for others to quickly grasp the reasonings behind less obvious things? src/tests/hdfs_tests.cpp (lines 47 - 48) <https://reviews.apache.org/r/41384/#comment170180> This confused me initially as it seems this does not really apply. The tests serialize bash scripts into this file which emulates the hadoop client's logic while operating on the local filesystem. src/tests/hdfs_tests.cpp (line 64) <https://reviews.apache.org/r/41384/#comment170182> Missing test heading comments in this case actually seems to be fine to me, given the easy to grasp nature and the expected repetition when insisting on doing so for every single one of these. src/tests/hdfs_tests.cpp (line 69) <https://reviews.apache.org/r/41384/#comment170184> Shall we add a comment on that we are emulating the version call as that one is excersized by the HDFS factory, testing the client? src/tests/hdfs_tests.cpp (line 72) <https://reviews.apache.org/r/41384/#comment170183> Would it make sense to somehow point to the expected hdfs-client argument syntax for the `exists` call (and the others as well)? In other words, why are we using the fourth argument here? Its `strings::format("%s fs -test -e '%s'", hadoop, absolutePath(path));` that explains this magic. Should we be verbose about that in some comments here and for the other calls? - Till Toenshoff On Dec. 15, 2015, 1:27 a.m., Jie Yu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/41384/ > ----------------------------------------------------------- > > (Updated Dec. 15, 2015, 1:27 a.m.) > > > Review request for mesos, Bernd Mathiske, Ben Mahler, and Vinod Kone. > > > Bugs: MESOS-3951 > https://issues.apache.org/jira/browse/MESOS-3951 > > > Repository: mesos > > > Description > ------- > > Added tests for HDFS client. > > > Diffs > ----- > > src/Makefile.am 9762f8567e32d70b8df2d694a1fef5c692fc730e > src/tests/hdfs_tests.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/41384/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Jie Yu > >
