You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Jeff Coffler <je...@taltos.com> on 2017/10/24 18:04:45 UTC
Review Request 63253: Fixed DefaultExecutorTest for proper URI
handling on Windows.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63253/
-----------------------------------------------------------
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
-------
Fixed changes from Gaston (git commit 772c8f5 on 10/02/2017)
for proper operation on Windows. Changed to use new URI methods
introduced as part of MESOS-6705, and used powershell on Windows
to replace "test" command on Linux.
Diffs
-----
src/tests/default_executor_tests.cpp 5078bd4d70698f5cbd14c971fcecfd58f8467a04
Diff: https://reviews.apache.org/r/63253/diff/1/
Testing
-------
Ran mesos-tests on both Windows and Linux with no errors.
Specifically ran mesos-tests with `--gtest_filter=MesosContainerizer/DefaultExecutorTest.*` to verify that the modified tests ran properly.
Thanks,
Jeff Coffler
Re: Review Request 63253: Fixed DefaultExecutorTest for proper URI
handling on Windows.
Posted by Jeff Coffler <je...@taltos.com>.
> On Oct. 25, 2017, 5:25 p.m., Andrew Schwartzmeyer wrote:
> > src/tests/default_executor_tests.cpp
> > Line 1766 (original), 1772 (patched)
> > <https://reviews.apache.org/r/63253/diff/1/?file=1867546#file1867546line1779>
> >
> > Is it just me or did this indentation change?
I don't see that. It's indented two spaces, and it used to be indented two spaces. Do you see something different?
- Jeff
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63253/#review189210
-----------------------------------------------------------
On Oct. 24, 2017, 6:04 p.m., Jeff Coffler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63253/
> -----------------------------------------------------------
>
> (Updated Oct. 24, 2017, 6:04 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
> -------
>
> Fixed changes from Gaston (git commit 772c8f5 on 10/02/2017)
> for proper operation on Windows. Changed to use new URI methods
> introduced as part of MESOS-6705, and used powershell on Windows
> to replace "test" command on Linux.
>
>
> Diffs
> -----
>
> src/tests/default_executor_tests.cpp 5078bd4d70698f5cbd14c971fcecfd58f8467a04
>
>
> Diff: https://reviews.apache.org/r/63253/diff/1/
>
>
> Testing
> -------
>
> Ran mesos-tests on both Windows and Linux with no errors.
>
> Specifically ran mesos-tests with `--gtest_filter=MesosContainerizer/DefaultExecutorTest.*` to verify that the modified tests ran properly.
>
>
> Thanks,
>
> Jeff Coffler
>
>
Re: Review Request 63253: Fixed DefaultExecutorTest for proper URI
handling on Windows.
Posted by Andrew Schwartzmeyer <an...@schwartzmeyer.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63253/#review189210
-----------------------------------------------------------
src/tests/default_executor_tests.cpp
Lines 1764 (patched)
<https://reviews.apache.org/r/63253/#comment266181>
+1, I missed this.
src/tests/default_executor_tests.cpp
Lines 1766-1770 (patched)
<https://reviews.apache.org/r/63253/#comment266182>
Well, no but nothing will run `alpine` in a Windows container.
Looking at `docker_containerizer_tests.cpp`, they're all hard-coded to use `alpine`, so we should drop this change entirely for now. Another team member is working to enable the Docker unit tests on Windows (which will involve swapping from `alpine` to `nanoserver` on Windows).
src/tests/default_executor_tests.cpp
Line 1766 (original), 1772 (patched)
<https://reviews.apache.org/r/63253/#comment266183>
Is it just me or did this indentation change?
- Andrew Schwartzmeyer
On Oct. 24, 2017, 11:04 a.m., Jeff Coffler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63253/
> -----------------------------------------------------------
>
> (Updated Oct. 24, 2017, 11:04 a.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
> -------
>
> Fixed changes from Gaston (git commit 772c8f5 on 10/02/2017)
> for proper operation on Windows. Changed to use new URI methods
> introduced as part of MESOS-6705, and used powershell on Windows
> to replace "test" command on Linux.
>
>
> Diffs
> -----
>
> src/tests/default_executor_tests.cpp 5078bd4d70698f5cbd14c971fcecfd58f8467a04
>
>
> Diff: https://reviews.apache.org/r/63253/diff/1/
>
>
> Testing
> -------
>
> Ran mesos-tests on both Windows and Linux with no errors.
>
> Specifically ran mesos-tests with `--gtest_filter=MesosContainerizer/DefaultExecutorTest.*` to verify that the modified tests ran properly.
>
>
> Thanks,
>
> Jeff Coffler
>
>
Re: Review Request 63253: Fixed DefaultExecutorTest for proper URI
handling on Windows.
Posted by Jeff Coffler <je...@taltos.com>.
> On Oct. 25, 2017, 3:58 p.m., Gaston Kleiman wrote:
> > src/tests/default_executor_tests.cpp
> > Lines 1764 (patched)
> > <https://reviews.apache.org/r/63253/diff/1/?file=1867546#file1867546line1771>
> >
> > The indentation here looks strange.
This was removed since it's specific to Linux (alpine image).
> On Oct. 25, 2017, 3:58 p.m., Gaston Kleiman wrote:
> > src/tests/default_executor_tests.cpp
> > Lines 1766-1770 (patched)
> > <https://reviews.apache.org/r/63253/diff/1/?file=1867546#file1867546line1773>
> >
> > I don't think this will work inside an `alpine` Docker container, will it?
Removed since this isn't appropriate for alpine container. I missed that the first time, thanks for pointing it out!
- Jeff
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63253/#review189201
-----------------------------------------------------------
On Oct. 24, 2017, 6:04 p.m., Jeff Coffler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63253/
> -----------------------------------------------------------
>
> (Updated Oct. 24, 2017, 6:04 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
> -------
>
> Fixed changes from Gaston (git commit 772c8f5 on 10/02/2017)
> for proper operation on Windows. Changed to use new URI methods
> introduced as part of MESOS-6705, and used powershell on Windows
> to replace "test" command on Linux.
>
>
> Diffs
> -----
>
> src/tests/default_executor_tests.cpp 5078bd4d70698f5cbd14c971fcecfd58f8467a04
>
>
> Diff: https://reviews.apache.org/r/63253/diff/1/
>
>
> Testing
> -------
>
> Ran mesos-tests on both Windows and Linux with no errors.
>
> Specifically ran mesos-tests with `--gtest_filter=MesosContainerizer/DefaultExecutorTest.*` to verify that the modified tests ran properly.
>
>
> Thanks,
>
> Jeff Coffler
>
>
Re: Review Request 63253: Fixed DefaultExecutorTest for proper URI
handling on Windows.
Posted by Gaston Kleiman <ga...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63253/#review189201
-----------------------------------------------------------
src/tests/default_executor_tests.cpp
Lines 1764 (patched)
<https://reviews.apache.org/r/63253/#comment266158>
The indentation here looks strange.
src/tests/default_executor_tests.cpp
Lines 1766-1770 (patched)
<https://reviews.apache.org/r/63253/#comment266159>
I don't think this will work inside an `alpine` Docker container, will it?
- Gaston Kleiman
On Oct. 24, 2017, 11:04 a.m., Jeff Coffler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63253/
> -----------------------------------------------------------
>
> (Updated Oct. 24, 2017, 11:04 a.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
> -------
>
> Fixed changes from Gaston (git commit 772c8f5 on 10/02/2017)
> for proper operation on Windows. Changed to use new URI methods
> introduced as part of MESOS-6705, and used powershell on Windows
> to replace "test" command on Linux.
>
>
> Diffs
> -----
>
> src/tests/default_executor_tests.cpp 5078bd4d70698f5cbd14c971fcecfd58f8467a04
>
>
> Diff: https://reviews.apache.org/r/63253/diff/1/
>
>
> Testing
> -------
>
> Ran mesos-tests on both Windows and Linux with no errors.
>
> Specifically ran mesos-tests with `--gtest_filter=MesosContainerizer/DefaultExecutorTest.*` to verify that the modified tests ran properly.
>
>
> Thanks,
>
> Jeff Coffler
>
>
Re: Review Request 63253: Fixed DefaultExecutorTest for proper URI
handling on Windows.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63253/#review189066
-----------------------------------------------------------
FAIL: The file 'C:DCOSmesosbuild-outputlogsapply-review-60620-stdout.log' already exists.
All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/63253
- Mesos Reviewbot Windows
On Oct. 24, 2017, 6:04 p.m., Jeff Coffler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63253/
> -----------------------------------------------------------
>
> (Updated Oct. 24, 2017, 6:04 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
> -------
>
> Fixed changes from Gaston (git commit 772c8f5 on 10/02/2017)
> for proper operation on Windows. Changed to use new URI methods
> introduced as part of MESOS-6705, and used powershell on Windows
> to replace "test" command on Linux.
>
>
> Diffs
> -----
>
> src/tests/default_executor_tests.cpp 5078bd4d70698f5cbd14c971fcecfd58f8467a04
>
>
> Diff: https://reviews.apache.org/r/63253/diff/1/
>
>
> Testing
> -------
>
> Ran mesos-tests on both Windows and Linux with no errors.
>
> Specifically ran mesos-tests with `--gtest_filter=MesosContainerizer/DefaultExecutorTest.*` to verify that the modified tests ran properly.
>
>
> Thanks,
>
> Jeff Coffler
>
>
Re: Review Request 63253: Fixed DefaultExecutorTest for proper URI
handling on Windows.
Posted by Jeff Coffler <je...@taltos.com>.
> On Oct. 25, 2017, 4:38 p.m., Gaston Kleiman wrote:
> > src/tests/default_executor_tests.cpp
> > Lines 1650 (patched)
> > <https://reviews.apache.org/r/63253/diff/1/?file=1867546#file1867546line1653>
> >
> > This should be:
> >
> > `" if ((Get-Content testFile) -NotMatch 'pizza')"`
> >
> > To make sure that the file is copied to the sandbox, and that the task's working dir is the sandbox.
>
> Andrew Schwartzmeyer wrote:
> This leads into a larger problem on Windows: `shell` tasks are run under `cmd.exe`, which as a legacy app does not support long paths (actually, it doesn't even recognize them, it thinks they're UNC paths are bails), so it falls back to a temp directory. Until we've swapped the default shell on Windows from `cmd.exe` to `powershell.exe`, this is a problem. The task _is_ started in the sandbox, but `cmd.exe` then changes directory :(
>
> All that said, we went the route of using `testFilePath`, the full path to the file ensure its contents are correct (that the file was copied right). But you're right, I think it makes more sense to ensure the task was started in the right place, and then test it relatively.
>
> Jeff, this means to go back to using `set_shell(false)` etc. to `powershell.exe` directly, instead of under `cmd.exe`.
Yup, I knew that when I discussed this with Gaston. All set with this.
- Jeff
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63253/#review189204
-----------------------------------------------------------
On Oct. 24, 2017, 6:04 p.m., Jeff Coffler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63253/
> -----------------------------------------------------------
>
> (Updated Oct. 24, 2017, 6:04 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
> -------
>
> Fixed changes from Gaston (git commit 772c8f5 on 10/02/2017)
> for proper operation on Windows. Changed to use new URI methods
> introduced as part of MESOS-6705, and used powershell on Windows
> to replace "test" command on Linux.
>
>
> Diffs
> -----
>
> src/tests/default_executor_tests.cpp 5078bd4d70698f5cbd14c971fcecfd58f8467a04
>
>
> Diff: https://reviews.apache.org/r/63253/diff/1/
>
>
> Testing
> -------
>
> Ran mesos-tests on both Windows and Linux with no errors.
>
> Specifically ran mesos-tests with `--gtest_filter=MesosContainerizer/DefaultExecutorTest.*` to verify that the modified tests ran properly.
>
>
> Thanks,
>
> Jeff Coffler
>
>
Re: Review Request 63253: Fixed DefaultExecutorTest for proper URI
handling on Windows.
Posted by Gaston Kleiman <ga...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63253/#review189204
-----------------------------------------------------------
src/tests/default_executor_tests.cpp
Lines 1650 (patched)
<https://reviews.apache.org/r/63253/#comment266166>
This should be:
`" if ((Get-Content testFile) -NotMatch 'pizza')"`
To make sure that the file is copied to the sandbox, and that the task's working dir is the sandbox.
- Gaston Kleiman
On Oct. 24, 2017, 11:04 a.m., Jeff Coffler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63253/
> -----------------------------------------------------------
>
> (Updated Oct. 24, 2017, 11:04 a.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
> -------
>
> Fixed changes from Gaston (git commit 772c8f5 on 10/02/2017)
> for proper operation on Windows. Changed to use new URI methods
> introduced as part of MESOS-6705, and used powershell on Windows
> to replace "test" command on Linux.
>
>
> Diffs
> -----
>
> src/tests/default_executor_tests.cpp 5078bd4d70698f5cbd14c971fcecfd58f8467a04
>
>
> Diff: https://reviews.apache.org/r/63253/diff/1/
>
>
> Testing
> -------
>
> Ran mesos-tests on both Windows and Linux with no errors.
>
> Specifically ran mesos-tests with `--gtest_filter=MesosContainerizer/DefaultExecutorTest.*` to verify that the modified tests ran properly.
>
>
> Thanks,
>
> Jeff Coffler
>
>
Re: Review Request 63253: Fixed DefaultExecutorTest for proper URI
handling on Windows.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63253/#review189127
-----------------------------------------------------------
Patch looks great!
Reviews applied: [60620, 60621, 60622, 60623, 60624, 60626, 60628, 63253]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos Reviewbot
On Oct. 24, 2017, 6:04 p.m., Jeff Coffler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63253/
> -----------------------------------------------------------
>
> (Updated Oct. 24, 2017, 6:04 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
> -------
>
> Fixed changes from Gaston (git commit 772c8f5 on 10/02/2017)
> for proper operation on Windows. Changed to use new URI methods
> introduced as part of MESOS-6705, and used powershell on Windows
> to replace "test" command on Linux.
>
>
> Diffs
> -----
>
> src/tests/default_executor_tests.cpp 5078bd4d70698f5cbd14c971fcecfd58f8467a04
>
>
> Diff: https://reviews.apache.org/r/63253/diff/1/
>
>
> Testing
> -------
>
> Ran mesos-tests on both Windows and Linux with no errors.
>
> Specifically ran mesos-tests with `--gtest_filter=MesosContainerizer/DefaultExecutorTest.*` to verify that the modified tests ran properly.
>
>
> Thanks,
>
> Jeff Coffler
>
>
Re: Review Request 63253: Fixed DefaultExecutorTest for proper URI
handling on Windows.
Posted by Andrew Schwartzmeyer <an...@schwartzmeyer.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63253/#review189083
-----------------------------------------------------------
Ship it!
Ship It!
- Andrew Schwartzmeyer
On Oct. 24, 2017, 11:04 a.m., Jeff Coffler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63253/
> -----------------------------------------------------------
>
> (Updated Oct. 24, 2017, 11:04 a.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
> -------
>
> Fixed changes from Gaston (git commit 772c8f5 on 10/02/2017)
> for proper operation on Windows. Changed to use new URI methods
> introduced as part of MESOS-6705, and used powershell on Windows
> to replace "test" command on Linux.
>
>
> Diffs
> -----
>
> src/tests/default_executor_tests.cpp 5078bd4d70698f5cbd14c971fcecfd58f8467a04
>
>
> Diff: https://reviews.apache.org/r/63253/diff/1/
>
>
> Testing
> -------
>
> Ran mesos-tests on both Windows and Linux with no errors.
>
> Specifically ran mesos-tests with `--gtest_filter=MesosContainerizer/DefaultExecutorTest.*` to verify that the modified tests ran properly.
>
>
> Thanks,
>
> Jeff Coffler
>
>
Re: Review Request 63253: Fixed DefaultExecutorTest for proper URI
handling on Windows.
Posted by Jeff Coffler <je...@taltos.com>.
> On Oct. 25, 2017, 10:07 p.m., Gaston Kleiman wrote:
> > src/tests/default_executor_tests.cpp
> > Lines 1657 (patched)
> > <https://reviews.apache.org/r/63253/diff/2/?file=1868878#file1868878line1660>
> >
> > Nit: do we need this the whitespace after the opening quotes?
There used to be a line before that (to invoke powershell), but it's gone now. Space removed after the opening quotes, and removed one space from the line below.
- Jeff
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63253/#review189252
-----------------------------------------------------------
On Oct. 25, 2017, 7:59 p.m., Jeff Coffler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63253/
> -----------------------------------------------------------
>
> (Updated Oct. 25, 2017, 7:59 p.m.)
>
>
> Review request for mesos, Andrew Schwartzmeyer, Gaston Kleiman, John Kordich, Joseph Wu, and Li Li.
>
>
> Bugs: MESOS-6705
> https://issues.apache.org/jira/browse/MESOS-6705
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Fixed changes from Gaston (git commit 772c8f5 on 10/02/2017)
> for proper operation on Windows. Changed to use new URI methods
> introduced as part of MESOS-6705, and used powershell on Windows
> to replace "test" command on Linux.
>
>
> Diffs
> -----
>
> src/tests/default_executor_tests.cpp 5078bd4d70698f5cbd14c971fcecfd58f8467a04
>
>
> Diff: https://reviews.apache.org/r/63253/diff/2/
>
>
> Testing
> -------
>
> Ran mesos-tests on both Windows and Linux with no errors.
>
> Specifically ran mesos-tests with `--gtest_filter=MesosContainerizer/DefaultExecutorTest.*` to verify that the modified tests ran properly.
>
>
> Thanks,
>
> Jeff Coffler
>
>
Re: Review Request 63253: Fixed DefaultExecutorTest for proper URI
handling on Windows.
Posted by Gaston Kleiman <ga...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63253/#review189252
-----------------------------------------------------------
Fix it, then Ship it!
src/tests/default_executor_tests.cpp
Lines 1657 (patched)
<https://reviews.apache.org/r/63253/#comment266268>
Nit: do we need this the whitespace after the opening quotes?
- Gaston Kleiman
On Oct. 25, 2017, 12:59 p.m., Jeff Coffler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63253/
> -----------------------------------------------------------
>
> (Updated Oct. 25, 2017, 12:59 p.m.)
>
>
> Review request for mesos, Andrew Schwartzmeyer, Gaston Kleiman, John Kordich, Joseph Wu, and Li Li.
>
>
> Bugs: MESOS-6705
> https://issues.apache.org/jira/browse/MESOS-6705
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Fixed changes from Gaston (git commit 772c8f5 on 10/02/2017)
> for proper operation on Windows. Changed to use new URI methods
> introduced as part of MESOS-6705, and used powershell on Windows
> to replace "test" command on Linux.
>
>
> Diffs
> -----
>
> src/tests/default_executor_tests.cpp 5078bd4d70698f5cbd14c971fcecfd58f8467a04
>
>
> Diff: https://reviews.apache.org/r/63253/diff/2/
>
>
> Testing
> -------
>
> Ran mesos-tests on both Windows and Linux with no errors.
>
> Specifically ran mesos-tests with `--gtest_filter=MesosContainerizer/DefaultExecutorTest.*` to verify that the modified tests ran properly.
>
>
> Thanks,
>
> Jeff Coffler
>
>
Re: Review Request 63253: Fixed DefaultExecutorTest for proper URI
handling on Windows.
Posted by Andrew Schwartzmeyer <an...@schwartzmeyer.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63253/#review189257
-----------------------------------------------------------
src/tests/default_executor_tests.cpp
Lines 1652-1653 (patched)
<https://reviews.apache.org/r/63253/#comment266270>
Nit: I would prefer these to be `powershell.exe` since we know the executable we're calling.
src/tests/default_executor_tests.cpp
Lines 1657-1658 (patched)
<https://reviews.apache.org/r/63253/#comment266271>
I think this could be one line now.
src/tests/default_executor_tests.cpp
Line 1693 (original), 1704 (patched)
<https://reviews.apache.org/r/63253/#comment266272>
Nit: s/coffer/coffler/g, s/powershell/PowerShell/g
- Andrew Schwartzmeyer
On Oct. 25, 2017, 3:17 p.m., Jeff Coffler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63253/
> -----------------------------------------------------------
>
> (Updated Oct. 25, 2017, 3:17 p.m.)
>
>
> Review request for mesos, Andrew Schwartzmeyer, Gaston Kleiman, John Kordich, Joseph Wu, and Li Li.
>
>
> Bugs: MESOS-6705
> https://issues.apache.org/jira/browse/MESOS-6705
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Fixed changes from Gaston (git commit 772c8f5 on 10/02/2017)
> for proper operation on Windows. Changed to use new URI methods
> introduced as part of MESOS-6705, and used powershell on Windows
> to replace "test" command on Linux.
>
>
> Diffs
> -----
>
> src/tests/default_executor_tests.cpp 5078bd4d70698f5cbd14c971fcecfd58f8467a04
>
>
> Diff: https://reviews.apache.org/r/63253/diff/3/
>
>
> Testing
> -------
>
> Ran mesos-tests on both Windows and Linux with no errors.
>
> Specifically ran mesos-tests with `--gtest_filter=MesosContainerizer/DefaultExecutorTest.*` to verify that the modified tests ran properly.
>
>
> Thanks,
>
> Jeff Coffler
>
>
Re: Review Request 63253: Fixed DefaultExecutorTest for proper URI
handling on Windows.
Posted by Andrew Schwartzmeyer <an...@schwartzmeyer.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63253/#review189257
-----------------------------------------------------------
src/tests/default_executor_tests.cpp
Lines 1652-1653 (patched)
<https://reviews.apache.org/r/63253/#comment266270>
Nit: I would prefer these to be `powershell.exe` since we know the executable we're calling.
src/tests/default_executor_tests.cpp
Lines 1657-1658 (patched)
<https://reviews.apache.org/r/63253/#comment266271>
I think this could be one line now.
src/tests/default_executor_tests.cpp
Line 1693 (original), 1704 (patched)
<https://reviews.apache.org/r/63253/#comment266272>
Nit: s/coffer/coffler/g, s/powershell/PowerShell/g
- Andrew Schwartzmeyer
On Oct. 25, 2017, 3:17 p.m., Jeff Coffler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63253/
> -----------------------------------------------------------
>
> (Updated Oct. 25, 2017, 3:17 p.m.)
>
>
> Review request for mesos, Andrew Schwartzmeyer, Gaston Kleiman, John Kordich, Joseph Wu, and Li Li.
>
>
> Bugs: MESOS-6705
> https://issues.apache.org/jira/browse/MESOS-6705
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Fixed changes from Gaston (git commit 772c8f5 on 10/02/2017)
> for proper operation on Windows. Changed to use new URI methods
> introduced as part of MESOS-6705, and used powershell on Windows
> to replace "test" command on Linux.
>
>
> Diffs
> -----
>
> src/tests/default_executor_tests.cpp 5078bd4d70698f5cbd14c971fcecfd58f8467a04
>
>
> Diff: https://reviews.apache.org/r/63253/diff/3/
>
>
> Testing
> -------
>
> Ran mesos-tests on both Windows and Linux with no errors.
>
> Specifically ran mesos-tests with `--gtest_filter=MesosContainerizer/DefaultExecutorTest.*` to verify that the modified tests ran properly.
>
>
> Thanks,
>
> Jeff Coffler
>
>
Re: Review Request 63253: Fixed DefaultExecutorTest for proper URI
handling on Windows.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63253/#review189326
-----------------------------------------------------------
Patch looks great!
Reviews applied: [60620, 60621, 60622, 60623, 60624, 60626, 60628, 63253]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos Reviewbot
On Oct. 25, 2017, 11:19 p.m., Jeff Coffler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63253/
> -----------------------------------------------------------
>
> (Updated Oct. 25, 2017, 11:19 p.m.)
>
>
> Review request for mesos, Andrew Schwartzmeyer, Gaston Kleiman, John Kordich, Joseph Wu, and Li Li.
>
>
> Bugs: MESOS-6705
> https://issues.apache.org/jira/browse/MESOS-6705
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Fixed changes from Gaston (git commit 772c8f5 on 10/02/2017)
> for proper operation on Windows. Changed to use new URI methods
> introduced as part of MESOS-6705, and used powershell on Windows
> to replace "test" command on Linux.
>
>
> Diffs
> -----
>
> src/tests/default_executor_tests.cpp 5078bd4d70698f5cbd14c971fcecfd58f8467a04
>
>
> Diff: https://reviews.apache.org/r/63253/diff/4/
>
>
> Testing
> -------
>
> Ran mesos-tests on both Windows and Linux with no errors.
>
> Specifically ran mesos-tests with `--gtest_filter=MesosContainerizer/DefaultExecutorTest.*` to verify that the modified tests ran properly.
>
>
> Thanks,
>
> Jeff Coffler
>
>
Re: Review Request 63253: Fixed DefaultExecutorTest for proper URI
handling on Windows.
Posted by Jeff Coffler <je...@taltos.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63253/
-----------------------------------------------------------
(Updated Oct. 25, 2017, 11:19 p.m.)
Review request for mesos, Andrew Schwartzmeyer, Gaston Kleiman, John Kordich, Joseph Wu, and Li Li.
Bugs: MESOS-6705
https://issues.apache.org/jira/browse/MESOS-6705
Repository: mesos
Description
-------
Fixed changes from Gaston (git commit 772c8f5 on 10/02/2017)
for proper operation on Windows. Changed to use new URI methods
introduced as part of MESOS-6705, and used powershell on Windows
to replace "test" command on Linux.
Diffs (updated)
-----
src/tests/default_executor_tests.cpp 5078bd4d70698f5cbd14c971fcecfd58f8467a04
Diff: https://reviews.apache.org/r/63253/diff/4/
Changes: https://reviews.apache.org/r/63253/diff/3-4/
Testing
-------
Ran mesos-tests on both Windows and Linux with no errors.
Specifically ran mesos-tests with `--gtest_filter=MesosContainerizer/DefaultExecutorTest.*` to verify that the modified tests ran properly.
Thanks,
Jeff Coffler
Re: Review Request 63253: Fixed DefaultExecutorTest for proper URI
handling on Windows.
Posted by Jeff Coffler <je...@taltos.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63253/
-----------------------------------------------------------
(Updated Oct. 25, 2017, 10:17 p.m.)
Review request for mesos, Andrew Schwartzmeyer, Gaston Kleiman, John Kordich, Joseph Wu, and Li Li.
Bugs: MESOS-6705
https://issues.apache.org/jira/browse/MESOS-6705
Repository: mesos
Description
-------
Fixed changes from Gaston (git commit 772c8f5 on 10/02/2017)
for proper operation on Windows. Changed to use new URI methods
introduced as part of MESOS-6705, and used powershell on Windows
to replace "test" command on Linux.
Diffs (updated)
-----
src/tests/default_executor_tests.cpp 5078bd4d70698f5cbd14c971fcecfd58f8467a04
Diff: https://reviews.apache.org/r/63253/diff/3/
Changes: https://reviews.apache.org/r/63253/diff/2-3/
Testing
-------
Ran mesos-tests on both Windows and Linux with no errors.
Specifically ran mesos-tests with `--gtest_filter=MesosContainerizer/DefaultExecutorTest.*` to verify that the modified tests ran properly.
Thanks,
Jeff Coffler
Re: Review Request 63253: Fixed DefaultExecutorTest for proper URI
handling on Windows.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63253/#review189251
-----------------------------------------------------------
PASS: Mesos patch 63253 was successfully built and tested.
Reviews applied: `['60620', '60621', '60622', '60623', '60624', '60626', '60628', '63253']`
All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/63253
- Mesos Reviewbot Windows
On Oct. 25, 2017, 7:59 p.m., Jeff Coffler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63253/
> -----------------------------------------------------------
>
> (Updated Oct. 25, 2017, 7:59 p.m.)
>
>
> Review request for mesos, Andrew Schwartzmeyer, Gaston Kleiman, John Kordich, Joseph Wu, and Li Li.
>
>
> Bugs: MESOS-6705
> https://issues.apache.org/jira/browse/MESOS-6705
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Fixed changes from Gaston (git commit 772c8f5 on 10/02/2017)
> for proper operation on Windows. Changed to use new URI methods
> introduced as part of MESOS-6705, and used powershell on Windows
> to replace "test" command on Linux.
>
>
> Diffs
> -----
>
> src/tests/default_executor_tests.cpp 5078bd4d70698f5cbd14c971fcecfd58f8467a04
>
>
> Diff: https://reviews.apache.org/r/63253/diff/2/
>
>
> Testing
> -------
>
> Ran mesos-tests on both Windows and Linux with no errors.
>
> Specifically ran mesos-tests with `--gtest_filter=MesosContainerizer/DefaultExecutorTest.*` to verify that the modified tests ran properly.
>
>
> Thanks,
>
> Jeff Coffler
>
>
Re: Review Request 63253: Fixed DefaultExecutorTest for proper URI
handling on Windows.
Posted by Jeff Coffler <je...@taltos.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63253/
-----------------------------------------------------------
(Updated Oct. 25, 2017, 7:59 p.m.)
Review request for mesos, Andrew Schwartzmeyer, Gaston Kleiman, John Kordich, Joseph Wu, and Li Li.
Bugs: MESOS-6705
https://issues.apache.org/jira/browse/MESOS-6705
Repository: mesos
Description
-------
Fixed changes from Gaston (git commit 772c8f5 on 10/02/2017)
for proper operation on Windows. Changed to use new URI methods
introduced as part of MESOS-6705, and used powershell on Windows
to replace "test" command on Linux.
Diffs (updated)
-----
src/tests/default_executor_tests.cpp 5078bd4d70698f5cbd14c971fcecfd58f8467a04
Diff: https://reviews.apache.org/r/63253/diff/2/
Changes: https://reviews.apache.org/r/63253/diff/1-2/
Testing
-------
Ran mesos-tests on both Windows and Linux with no errors.
Specifically ran mesos-tests with `--gtest_filter=MesosContainerizer/DefaultExecutorTest.*` to verify that the modified tests ran properly.
Thanks,
Jeff Coffler
Re: Review Request 63253: Fixed DefaultExecutorTest for proper URI
handling on Windows.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63253/#review189194
-----------------------------------------------------------
PASS: Mesos patch 63253 was successfully built and tested.
Reviews applied: `['60620', '60621', '60622', '60623', '60624', '60626', '60628', '63253']`
All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/63253
- Mesos Reviewbot Windows
On Oct. 24, 2017, 6:04 p.m., Jeff Coffler wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63253/
> -----------------------------------------------------------
>
> (Updated Oct. 24, 2017, 6:04 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
> -------
>
> Fixed changes from Gaston (git commit 772c8f5 on 10/02/2017)
> for proper operation on Windows. Changed to use new URI methods
> introduced as part of MESOS-6705, and used powershell on Windows
> to replace "test" command on Linux.
>
>
> Diffs
> -----
>
> src/tests/default_executor_tests.cpp 5078bd4d70698f5cbd14c971fcecfd58f8467a04
>
>
> Diff: https://reviews.apache.org/r/63253/diff/1/
>
>
> Testing
> -------
>
> Ran mesos-tests on both Windows and Linux with no errors.
>
> Specifically ran mesos-tests with `--gtest_filter=MesosContainerizer/DefaultExecutorTest.*` to verify that the modified tests ran properly.
>
>
> Thanks,
>
> Jeff Coffler
>
>