----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45057/#review124422 -----------------------------------------------------------
Fix it, then Ship it! Thanks for the patch! This looks great! I made some comments on the first test, but the same comments should apply to other tests as well. Once the path is updated, I'll commit it. Thanks! src/tests/fetcher_tests.cpp (line 642) <https://reviews.apache.org/r/45057/#comment186968> 2 lines apart src/tests/fetcher_tests.cpp (lines 647 - 649) <https://reviews.apache.org/r/45057/#comment186969> Can you create a temp file under os::getcwd()? The TemporaryDirectoryTest fixture will cleanup the current working directory after the test finishes/failed. If the test fails, the existing code will leak the tmp zip file. src/tests/fetcher_tests.cpp (line 648) <https://reviews.apache.org/r/45057/#comment186972> Kill this line. src/tests/fetcher_tests.cpp (lines 657 - 660) <https://reviews.apache.org/r/45057/#comment186970> Indentation here should be 4: ``` ASSERT_SOME(... "UES....." "...." "...").get())); ``` src/tests/fetcher_tests.cpp (line 661) <https://reviews.apache.org/r/45057/#comment186971> Insert a new line above. We typically insert a new line after a multi-line statement. src/tests/fetcher_tests.cpp (line 682) <https://reviews.apache.org/r/45057/#comment186973> s/extract/extractedFile/ Please use path::join(os::getcwd(), "hello"); src/tests/fetcher_tests.cpp (line 687) <https://reviews.apache.org/r/45057/#comment186974> This is not necessary if you put the zip file under os::getcwd() as I mentioned above. src/tests/fetcher_tests.cpp (line 689) <https://reviews.apache.org/r/45057/#comment186975> 2 lines apart please. src/tests/fetcher_tests.cpp (line 694) <https://reviews.apache.org/r/45057/#comment186976> Ditto. Kill this line. - Jie Yu On March 19, 2016, 10:26 a.m., Tomasz Janiszewski wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45057/ > ----------------------------------------------------------- > > (Updated March 19, 2016, 10:26 a.m.) > > > Review request for mesos and Jie Yu. > > > Bugs: MESOS-4885 > https://issues.apache.org/jira/browse/MESOS-4885 > > > Repository: mesos > > > Description > ------- > > Made `unzip` overwrite existing files without prompting. > > > Diffs > ----- > > src/launcher/fetcher.cpp f85b118fb19cf9d4563f89847a783be35067e815 > src/tests/fetcher_tests.cpp fb47706eb90ae5808bafe13c681d609a808b0c8e > > Diff: https://reviews.apache.org/r/45057/diff/ > > > Testing > ------- > > > Thanks, > > Tomasz Janiszewski > >
