Re: Review Request 60628: Enable fetcher_tests.cpp unit test module on Windows platform.

[Andrew Schwartzmeyer <an...@schwartzmeyer.com>]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60628/#review182978
-----------------------------------------------------------




src/launcher/CMakeLists.txt
Lines 38-65 (original), 38-57 (patched)
<https://reviews.apache.org/r/60628/#comment258962>

    I rewrote this file. Poor Joe :(



src/slave/containerizer/fetcher.cpp
Line 780 (original), 778-783 (patched)
<https://reviews.apache.org/r/60628/#comment258953>

    I don't think this change is necessary. On Windows, when this eventually reaches `CreateProcess`, it tries the various suffixes such as `.exe` to launch the process.
    
    Furthermore, we don't do this anywhere else, so it's inconsistent.



src/tests/fetcher_tests.cpp
Line 355 (original), 359 (patched)
<https://reviews.apache.org/r/60628/#comment258954>

    Ah. Precisely one use of this odd boolean flag to `path::uri`. I would definitely suggest this just be `path::normalize`. Or make `path::uri` idempotent (i.e. only add the prefix if it's not already there, if that's the logic you're needing here.).



src/tests/fetcher_tests.cpp
Line 466 (original), 470 (patched)
<https://reviews.apache.org/r/60628/#comment258956>

    Huh?



src/tests/fetcher_tests.cpp
Line 512 (original), 516 (patched)
<https://reviews.apache.org/r/60628/#comment258963>

    Another, huh?



src/tests/fetcher_tests.cpp
Line 565 (original), 569 (patched)
<https://reviews.apache.org/r/60628/#comment258964>

    This looks like it dropped the `localhost` part.



src/tests/fetcher_tests.cpp
Line 593 (original), 598 (patched)
<https://reviews.apache.org/r/60628/#comment258965>

    Good place for `path::normalize`.



src/tests/fetcher_tests.cpp
Line 739 (original), 754 (patched)
<https://reviews.apache.org/r/60628/#comment258966>

    Ditto.



src/tests/fetcher_tests.cpp
Line 827 (original), 845 (patched)
<https://reviews.apache.org/r/60628/#comment258967>

    Ditto.



src/tests/fetcher_tests.cpp
Line 880 (original), 898 (patched)
<https://reviews.apache.org/r/60628/#comment258968>

    Ditto.



src/tests/fetcher_tests.cpp
Line 937 (original), 969 (patched)
<https://reviews.apache.org/r/60628/#comment258969>

    Ditto.



src/tests/fetcher_tests.cpp
Line 979 (original), 1011 (patched)
<https://reviews.apache.org/r/60628/#comment258970>

    Ditto.


- Andrew Schwartzmeyer


On July 3, 2017, 12:35 p.m., Jeff Coffler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60628/
> -----------------------------------------------------------
> 
> (Updated July 3, 2017, 12:35 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and Li Li.
> 
> 
> Bugs: MESOS-6705
>     https://issues.apache.org/jira/browse/MESOS-6705
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Document that some FetcherTest tests won't work on Windows platform.
>   Tests for tar, gzip, and such won't be working on Windows for
>   the time being. Thoughts are to provide this capability to the
>   Fetcher in a cross-platform manner via a programmatic code library
>   rather than Linux-specific command line tools (tar, gzip, etc).
> 
>   In the short term, zip and unzip will work since PowerShell can
>   support that natively.
> 
> Fix FetcherTest.AbsoluteCustomSubdirectoryFails on Windows platform.
> Test FetcherTest.InvalidUser can't work on Windows for now.
> Fix test FetcherTest.NonExistingFile on Windows platform.
> Fix test FetcherTest.AbsolutePath on Windows platform.
> Fix test FetcherTest.RelativeFilePath on Windows platform.
> Fix test FetcherTest.OSNetUriTest on Windows platform.
> Fix test FetcherTest.OSNetUriSpaceTest on Windows platform.
> Fix test FetcherTest.FileLocalhostURI on Windows platform.
> Fix test FetcherTest.Unzip_ExtractFile on Windows platform.
> Fix test FetcherTest.Unzip_ExtractInvalidFile on Windows platform.
> Fix test Unzip_ExtractFileWithDuplicatedEntries on Windows platform.
> Fix test FetcherTest.UseCustomOutputFile on Windows platform.
> 
> 
> Diffs
> -----
> 
>   src/launcher/CMakeLists.txt 97edc4a28a995da8a5e963aaa62c4c955cc59719 
>   src/launcher/fetcher.cpp 42980f5a4a40b72f754156469e9fe60a952d1d87 
>   src/slave/containerizer/fetcher.cpp e3c786b36ad16b33ef9d3eed15f722890e80f0bb 
>   src/tests/fetcher_tests.cpp 99149baa1c7abfabf572a0d0f4512a8e84d1e5be 
> 
> 
> Diff: https://reviews.apache.org/r/60628/diff/1/
> 
> 
> Testing
> -------
> 
> See upstream.
> 
> 
> Thanks,
> 
> Jeff Coffler
> 
>