Re: Review Request 58720: Extended the unit test infrastructure in the new Mesos CLI.

[Mesos Reviewbot Windows]

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



Patch looks great!

Reviews applied: [59634, 58720]

Passed command: support\windows-build.bat

- Mesos Reviewbot Windows


On June 12, 2017, 8:49 a.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58720/
> ---
> 
> (Updated June 12, 2017, 8:49 a.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-7283
> https://issues.apache.org/jira/browse/MESOS-7283
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This infrastructure includes the ability to bring up a test cluster to
> run the CLI against. Future unit tests will use this infrastructure to
> test their commands against a running mesos cluster. The tests require
> some binaries created when building Mesos.
> 
> 
> Diffs
> -
> 
>   src/cli_new/lib/cli/tests/__init__.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/base.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/constants.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/tests.py PRE-CREATION 
>   src/cli_new/pip-requirements.txt 28613e56a5c6d5c7606a7e58d6125b0c34748e83 
>   src/cli_new/tests/main.py dff5d48b0ddae87960a78f9d05e4ae597912f1f6 
> 
> 
> Diff: https://reviews.apache.org/r/58720/diff/16/
> 
> 
> Testing
> ---
> 
> PEP8 and Pylint used to make sure that the code style is correct. Manuel test:
> 
> $ cd src/cli_new
> $ ./bootstrap
> $ source activate
> 
> (mesos-cli) $ mesos-cli-tests
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 58720: Extended the unit test infrastructure in the new Mesos CLI.

[Kevin Klues]

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


Ship it!




Ship It!

- Kevin Klues


On June 12, 2017, 3:49 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58720/
> ---
> 
> (Updated June 12, 2017, 3:49 p.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-7283
> https://issues.apache.org/jira/browse/MESOS-7283
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This infrastructure includes the ability to bring up a test cluster to
> run the CLI against. Future unit tests will use this infrastructure to
> test their commands against a running mesos cluster. The tests require
> some binaries created when building Mesos.
> 
> 
> Diffs
> -
> 
>   src/cli_new/lib/cli/tests/__init__.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/base.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/constants.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/tests.py PRE-CREATION 
>   src/cli_new/pip-requirements.txt 28613e56a5c6d5c7606a7e58d6125b0c34748e83 
>   src/cli_new/tests/main.py dff5d48b0ddae87960a78f9d05e4ae597912f1f6 
> 
> 
> Diff: https://reviews.apache.org/r/58720/diff/16/
> 
> 
> Testing
> ---
> 
> PEP8 and Pylint used to make sure that the code style is correct. Manuel test:
> 
> $ cd src/cli_new
> $ ./bootstrap
> $ source activate
> 
> (mesos-cli) $ mesos-cli-tests
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 58720: Extended the unit test infrastructure in the new Mesos CLI.

[Kevin Klues]

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




src/cli_new/lib/cli/tests/base.py
Lines 216 (patched)


I had to change this line as:
```
diff --git a/src/cli_new/lib/cli/tests/base.py 
b/src/cli_new/lib/cli/tests/base.py
index 94cbb0879..62a4ee8ac 100644
--- a/src/cli_new/lib/cli/tests/base.py
+++ b/src/cli_new/lib/cli/tests/base.py
@@ -213,7 +213,7 @@ class Agent(Executable):
 if "runtime_dir" not in flags:
 flags["runtime_dir"] = tempfile.mkdtemp()
 # Disabling systemd support on Linux to run without sudo.
-if sys.platform == "linux" and "systemd_enable_support" not in 
flags:
+if "linux" in sys.platform and "systemd_enable_support" not in 
flags:
 flags["systemd_enable_support"] = "false"
 
 self.flags = flags
```

I will simply commit this witht his change in leiu of doing another round 
of reviews.

Thanks for beig so patient!


- Kevin Klues


On June 12, 2017, 3:49 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58720/
> ---
> 
> (Updated June 12, 2017, 3:49 p.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-7283
> https://issues.apache.org/jira/browse/MESOS-7283
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This infrastructure includes the ability to bring up a test cluster to
> run the CLI against. Future unit tests will use this infrastructure to
> test their commands against a running mesos cluster. The tests require
> some binaries created when building Mesos.
> 
> 
> Diffs
> -
> 
>   src/cli_new/lib/cli/tests/__init__.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/base.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/constants.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/tests.py PRE-CREATION 
>   src/cli_new/pip-requirements.txt 28613e56a5c6d5c7606a7e58d6125b0c34748e83 
>   src/cli_new/tests/main.py dff5d48b0ddae87960a78f9d05e4ae597912f1f6 
> 
> 
> Diff: https://reviews.apache.org/r/58720/diff/16/
> 
> 
> Testing
> ---
> 
> PEP8 and Pylint used to make sure that the code style is correct. Manuel test:
> 
> $ cd src/cli_new
> $ ./bootstrap
> $ source activate
> 
> (mesos-cli) $ mesos-cli-tests
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 58720: Extended the unit test infrastructure in the new Mesos CLI.

[Armand Grillet]

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

(Updated June 12, 2017, 3:49 p.m.)


Review request for mesos and Kevin Klues.


Changes
---

Updated the 'depends on field' so the chain gets pulled in correctly.


Bugs: MESOS-7283
https://issues.apache.org/jira/browse/MESOS-7283


Repository: mesos


Description
---

This infrastructure includes the ability to bring up a test cluster to
run the CLI against. Future unit tests will use this infrastructure to
test their commands against a running mesos cluster. The tests require
some binaries created when building Mesos.


Diffs
-

  src/cli_new/lib/cli/tests/__init__.py PRE-CREATION 
  src/cli_new/lib/cli/tests/base.py PRE-CREATION 
  src/cli_new/lib/cli/tests/constants.py PRE-CREATION 
  src/cli_new/lib/cli/tests/tests.py PRE-CREATION 
  src/cli_new/pip-requirements.txt 28613e56a5c6d5c7606a7e58d6125b0c34748e83 
  src/cli_new/tests/main.py dff5d48b0ddae87960a78f9d05e4ae597912f1f6 


Diff: https://reviews.apache.org/r/58720/diff/16/


Testing
---

PEP8 and Pylint used to make sure that the code style is correct. Manuel test:

$ cd src/cli_new
$ ./bootstrap
$ source activate

(mesos-cli) $ mesos-cli-tests


Thanks,

Armand Grillet

Re: Review Request 58720: Extended the unit test infrastructure in the new Mesos CLI.

[Mesos Reviewbot Windows]

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



Patch looks great!

Reviews applied: [58720]

Passed command: support\windows-build.bat

- Mesos Reviewbot Windows


On May 31, 2017, 2:10 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58720/
> ---
> 
> (Updated May 31, 2017, 2:10 p.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-7283
> https://issues.apache.org/jira/browse/MESOS-7283
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This infrastructure includes the ability to bring up a test cluster to
> run the CLI against. Future unit tests will use this infrastructure to
> test their commands against a running mesos cluster. The tests require
> some binaries created when building Mesos.
> 
> 
> Diffs
> -
> 
>   src/cli_new/lib/cli/tests/__init__.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/base.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/constants.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/tests.py PRE-CREATION 
>   src/cli_new/pip-requirements.txt 28613e56a5c6d5c7606a7e58d6125b0c34748e83 
>   src/cli_new/tests/main.py dff5d48b0ddae87960a78f9d05e4ae597912f1f6 
> 
> 
> Diff: https://reviews.apache.org/r/58720/diff/16/
> 
> 
> Testing
> ---
> 
> PEP8 and Pylint used to make sure that the code style is correct. Manuel test:
> 
> $ cd src/cli_new
> $ ./bootstrap
> $ source activate
> 
> (mesos-cli) $ mesos-cli-tests
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 58720: Extended the unit test infrastructure in the new Mesos CLI.

[Armand Grillet]

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

(Updated May 31, 2017, 12:10 p.m.)


Review request for mesos and Kevin Klues.


Changes
---

Last issue fixed and code rebased.


Bugs: MESOS-7283
https://issues.apache.org/jira/browse/MESOS-7283


Repository: mesos


Description
---

This infrastructure includes the ability to bring up a test cluster to
run the CLI against. Future unit tests will use this infrastructure to
test their commands against a running mesos cluster. The tests require
some binaries created when building Mesos.


Diffs (updated)
-

  src/cli_new/lib/cli/tests/__init__.py PRE-CREATION 
  src/cli_new/lib/cli/tests/base.py PRE-CREATION 
  src/cli_new/lib/cli/tests/constants.py PRE-CREATION 
  src/cli_new/lib/cli/tests/tests.py PRE-CREATION 
  src/cli_new/pip-requirements.txt 28613e56a5c6d5c7606a7e58d6125b0c34748e83 
  src/cli_new/tests/main.py dff5d48b0ddae87960a78f9d05e4ae597912f1f6 


Diff: https://reviews.apache.org/r/58720/diff/16/

Changes: https://reviews.apache.org/r/58720/diff/15-16/


Testing
---

PEP8 and Pylint used to make sure that the code style is correct. Manuel test:

$ cd src/cli_new
$ ./bootstrap
$ source activate

(mesos-cli) $ mesos-cli-tests


Thanks,

Armand Grillet

Re: Review Request 58720: Extended the unit test infrastructure in the new Mesos CLI.

[Mesos Reviewbot Windows]

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



Patch looks great!

Reviews applied: [58720]

Passed command: support\windows-build.bat

- Mesos Reviewbot Windows


On May 30, 2017, 5:25 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58720/
> ---
> 
> (Updated May 30, 2017, 5:25 p.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-7283
> https://issues.apache.org/jira/browse/MESOS-7283
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This infrastructure includes the ability to bring up a test cluster to
> run the CLI against. Future unit tests will use this infrastructure to
> test their commands against a running mesos cluster. The tests require
> some binaries created when building Mesos.
> 
> 
> Diffs
> -
> 
>   src/cli_new/lib/cli/tests/__init__.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/base.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/constants.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/tests.py PRE-CREATION 
>   src/cli_new/pip-requirements.txt 28613e56a5c6d5c7606a7e58d6125b0c34748e83 
>   src/cli_new/tests/main.py dff5d48b0ddae87960a78f9d05e4ae597912f1f6 
> 
> 
> Diff: https://reviews.apache.org/r/58720/diff/15/
> 
> 
> Testing
> ---
> 
> PEP8 and Pylint used to make sure that the code style is correct. Manuel test:
> 
> $ cd src/cli_new
> $ ./bootstrap
> $ source activate
> 
> (mesos-cli) $ mesos-cli-tests
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 58720: Extended the unit test infrastructure in the new Mesos CLI.

[Kevin Klues]

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


Fix it, then Ship it!




I am just waiting on two things before committing this:
1) A ShipIt for https://reviews.apache.org/r/59634 so I can land that before 
this one.
2) Andrei, to either drop his issue or comment on why he thinks we should 
follow his suggestion.


src/cli_new/lib/cli/tests/base.py
Lines 216 (patched)


My `sys.platform` was `linux2`, so I changed this to:
```
# Disabling systemd support on Linux to run without sudo.
if "linux" in sys.platform and "systemd_enable_support" not in flags:
flags["systemd_enable_support"] = "false"
```

I will commit this without needing another round of reviews.


- Kevin Klues


On May 30, 2017, 5:25 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58720/
> ---
> 
> (Updated May 30, 2017, 5:25 p.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-7283
> https://issues.apache.org/jira/browse/MESOS-7283
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This infrastructure includes the ability to bring up a test cluster to
> run the CLI against. Future unit tests will use this infrastructure to
> test their commands against a running mesos cluster. The tests require
> some binaries created when building Mesos.
> 
> 
> Diffs
> -
> 
>   src/cli_new/lib/cli/tests/__init__.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/base.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/constants.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/tests.py PRE-CREATION 
>   src/cli_new/pip-requirements.txt 28613e56a5c6d5c7606a7e58d6125b0c34748e83 
>   src/cli_new/tests/main.py dff5d48b0ddae87960a78f9d05e4ae597912f1f6 
> 
> 
> Diff: https://reviews.apache.org/r/58720/diff/15/
> 
> 
> Testing
> ---
> 
> PEP8 and Pylint used to make sure that the code style is correct. Manuel test:
> 
> $ cd src/cli_new
> $ ./bootstrap
> $ source activate
> 
> (mesos-cli) $ mesos-cli-tests
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 58720: Extended the unit test infrastructure in the new Mesos CLI.

[Kevin Klues]

> On May 7, 2017, 8:13 p.m., Kevin Klues wrote:
> > src/cli_new/lib/cli/tests/base.py
> > Lines 384-401 (patched)
> > 
> >
> > This is fine for now, but I'm wondering if we can't find a more generic 
> > way of discovering the build directorty.  This current method assumes we 
> > always create the build directory in a specific location.
> > 
> > It also assumes we have a build directory at all. Which maybe we should 
> > safeguard againts.
> 
> Armand Grillet wrote:
> True, we could add an option like for our configure script with 
> `--with-mesos-build-dir`. Our script mesos-cli-tests is currently small and 
> supporting long command line options is gonna require a lot of code (it does 
> not work out of the box on macOS). The easiest solution would be to have a 
> short command line option to specify your build path (e.g. `mesos-cli-tests 
> -b=path/to/mesos/build/dir`).
> 
> Kevin Klues wrote:
> Since this sounds like a configuration time thing, I'd rather not have 
> this function living in this `tests.py` file. We should let it be set to the 
> default as you have, but also allow it to be passed in from above somehow.
> 
> I'm thinking, we could move it into `CLITestCase` as a static method and 
> rename it to `default_mesos_build_directory()`. We can then have a class 
> variable called `MESOS_BUILD_DIRECTORY` which is initially set to 
> `default_mesos_build_directory()`, but can be overriden (if desired) in 
> `src/cli_new/tests/main.py`.
> 
> Something like:
> ```
> class CLITestCase(unittest.TestCase):
> """
> Base class for CLI TestCases.
> """
> @classmethod
> def setUpClass(cls):
> print "\n{class_name}".format(class_name=cls.__name__)
> 
> @staticmethod
> def default_mesos_build_directory():
> """
> Returns the default path of the Mesos build directory. Useful when
> we wish to use some binaries such as mesos-execute.
> """
> tests_dir = os.path.dirname(__file__)
> cli_dir = os.path.dirname(tests_dir)
> lib_dir = os.path.dirname(cli_dir)
> cli_new_dir = os.path.dirname(lib_dir)
> src_dir = os.path.dirname(cli_new_dir)
> mesos_dir = os.path.dirname(src_dir)
> build_dir = os.path.join(mesos_dir, "build")
> 
> if os.path.isdir(build_dir):
> return build_dir
> else:
> raise CLIException("The Mesos build directory does not exist: 
> {path}"
>.format(path=build_dir))
> 
> CLITestCase.MESOS_BUILD_DIRECTORY = 
> CLITestCase.default_mesos_build_directory()
> ```
> 
> And then if we ever wanted to override it we could do (for example):
> ```
> # pylint: disable=unused-import
> # We import the tests that we want to run.
> from cli.tests import CLITestCase
> from cli.tests import TestInfrastructure
> 
> if __name__ == '__main__':
> CLITestCase.MESOS_BUILD_DIRECTORY = ...
> 
> print colored("Running the Mesos CLI unit tests", "yellow")
> unittest.main(verbosity=2, testRunner=unittest.TextTestRunner)
> ```
> 
> Armand Grillet wrote:
> I have followed your suggestion, in order to make the setting more 
> explicit `CLITestCase.MESOS_BUILD_DIR = 
> CLITestCase.default_mesos_build_dir()` is in `tests/main.py` and we only 
> instantiate the constant in `CLITestCase`. The instantiation has to be done 
> to avoid the Pylint error `Class 'CLITestCase' has no 'MESOS_BUILD_DIR' 
> member (no-member)`.

Perfect. That seems clearer anyway.


- Kevin


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


On May 30, 2017, 5:25 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58720/
> ---
> 
> (Updated May 30, 2017, 5:25 p.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-7283
> https://issues.apache.org/jira/browse/MESOS-7283
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This infrastructure includes the ability to bring up a test cluster to
> run the CLI against. Future unit tests will use this infrastructure to
> test their commands against a running mesos cluster. The tests require
> some binaries created when building Mesos.
> 
> 
> Diffs
> -
> 
>   src/cli_new/lib/cli/tests/__init__.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/base.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/constants.py PRE-

Re: Review Request 58720: Extended the unit test infrastructure in the new Mesos CLI.

[Armand Grillet]

> On May 7, 2017, 8:13 p.m., Kevin Klues wrote:
> > src/cli_new/lib/cli/tests/base.py
> > Lines 384-401 (patched)
> > 
> >
> > This is fine for now, but I'm wondering if we can't find a more generic 
> > way of discovering the build directorty.  This current method assumes we 
> > always create the build directory in a specific location.
> > 
> > It also assumes we have a build directory at all. Which maybe we should 
> > safeguard againts.
> 
> Armand Grillet wrote:
> True, we could add an option like for our configure script with 
> `--with-mesos-build-dir`. Our script mesos-cli-tests is currently small and 
> supporting long command line options is gonna require a lot of code (it does 
> not work out of the box on macOS). The easiest solution would be to have a 
> short command line option to specify your build path (e.g. `mesos-cli-tests 
> -b=path/to/mesos/build/dir`).
> 
> Kevin Klues wrote:
> Since this sounds like a configuration time thing, I'd rather not have 
> this function living in this `tests.py` file. We should let it be set to the 
> default as you have, but also allow it to be passed in from above somehow.
> 
> I'm thinking, we could move it into `CLITestCase` as a static method and 
> rename it to `default_mesos_build_directory()`. We can then have a class 
> variable called `MESOS_BUILD_DIRECTORY` which is initially set to 
> `default_mesos_build_directory()`, but can be overriden (if desired) in 
> `src/cli_new/tests/main.py`.
> 
> Something like:
> ```
> class CLITestCase(unittest.TestCase):
> """
> Base class for CLI TestCases.
> """
> @classmethod
> def setUpClass(cls):
> print "\n{class_name}".format(class_name=cls.__name__)
> 
> @staticmethod
> def default_mesos_build_directory():
> """
> Returns the default path of the Mesos build directory. Useful when
> we wish to use some binaries such as mesos-execute.
> """
> tests_dir = os.path.dirname(__file__)
> cli_dir = os.path.dirname(tests_dir)
> lib_dir = os.path.dirname(cli_dir)
> cli_new_dir = os.path.dirname(lib_dir)
> src_dir = os.path.dirname(cli_new_dir)
> mesos_dir = os.path.dirname(src_dir)
> build_dir = os.path.join(mesos_dir, "build")
> 
> if os.path.isdir(build_dir):
> return build_dir
> else:
> raise CLIException("The Mesos build directory does not exist: 
> {path}"
>.format(path=build_dir))
> 
> CLITestCase.MESOS_BUILD_DIRECTORY = 
> CLITestCase.default_mesos_build_directory()
> ```
> 
> And then if we ever wanted to override it we could do (for example):
> ```
> # pylint: disable=unused-import
> # We import the tests that we want to run.
> from cli.tests import CLITestCase
> from cli.tests import TestInfrastructure
> 
> if __name__ == '__main__':
> CLITestCase.MESOS_BUILD_DIRECTORY = ...
> 
> print colored("Running the Mesos CLI unit tests", "yellow")
> unittest.main(verbosity=2, testRunner=unittest.TextTestRunner)
> ```

I have followed your suggestion, in order to make the setting more explicit 
`CLITestCase.MESOS_BUILD_DIR = CLITestCase.default_mesos_build_dir()` is in 
`tests/main.py` and we only instantiate the constant in `CLITestCase`. The 
instantiation has to be done to avoid the Pylint error `Class 'CLITestCase' has 
no 'MESOS_BUILD_DIR' member (no-member)`.


- Armand


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


On May 30, 2017, 5:25 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58720/
> ---
> 
> (Updated May 30, 2017, 5:25 p.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-7283
> https://issues.apache.org/jira/browse/MESOS-7283
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This infrastructure includes the ability to bring up a test cluster to
> run the CLI against. Future unit tests will use this infrastructure to
> test their commands against a running mesos cluster. The tests require
> some binaries created when building Mesos.
> 
> 
> Diffs
> -
> 
>   src/cli_new/lib/cli/tests/__init__.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/base.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/constants.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/tests.py PRE-CREATION 
>   src/cli_new/

Re: Review Request 58720: Extended the unit test infrastructure in the new Mesos CLI.

[Armand Grillet]

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

(Updated May 30, 2017, 5:25 p.m.)


Review request for mesos and Kevin Klues.


Changes
---

Management of `MESOS_BUILD_DIR` improved.


Bugs: MESOS-7283
https://issues.apache.org/jira/browse/MESOS-7283


Repository: mesos


Description
---

This infrastructure includes the ability to bring up a test cluster to
run the CLI against. Future unit tests will use this infrastructure to
test their commands against a running mesos cluster. The tests require
some binaries created when building Mesos.


Diffs (updated)
-

  src/cli_new/lib/cli/tests/__init__.py PRE-CREATION 
  src/cli_new/lib/cli/tests/base.py PRE-CREATION 
  src/cli_new/lib/cli/tests/constants.py PRE-CREATION 
  src/cli_new/lib/cli/tests/tests.py PRE-CREATION 
  src/cli_new/pip-requirements.txt 28613e56a5c6d5c7606a7e58d6125b0c34748e83 
  src/cli_new/tests/main.py dff5d48b0ddae87960a78f9d05e4ae597912f1f6 


Diff: https://reviews.apache.org/r/58720/diff/15/

Changes: https://reviews.apache.org/r/58720/diff/14-15/


Testing
---

PEP8 and Pylint used to make sure that the code style is correct. Manuel test:

$ cd src/cli_new
$ ./bootstrap
$ source activate

(mesos-cli) $ mesos-cli-tests


Thanks,

Armand Grillet

Re: Review Request 58720: Extended the unit test infrastructure in the new Mesos CLI.

[Andrei Budnik]

> On May 23, 2017, 3:59 p.m., Andrei Budnik wrote:
> > src/cli_new/lib/cli/tests/base.py
> > Lines 52 (patched)
> > 
> >
> > I would prefer os.linesep to '\n' here and everywhere to avoid extra 
> > constant.
> 
> Kevin Klues wrote:
> The python documentation disagrees:
> 
> https://docs.python.org/3.0/library/os.html
> 
> ```
> os.linesep
> 
> The string used to separate (or, rather, terminate) lines on the current 
> platform. This may be a single character, such as '\n' for POSIX, or multiple 
> characters, for example, '\r\n' for Windows. Do not use os.linesep as a line 
> terminator when writing files opened in text mode (the default); use a single 
> '\n' instead, on all platforms.
> ```

Oh, I agree with you. Dropped the issue.


- Andrei


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


On May 30, 2017, 10 a.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58720/
> ---
> 
> (Updated May 30, 2017, 10 a.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-7283
> https://issues.apache.org/jira/browse/MESOS-7283
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This infrastructure includes the ability to bring up a test cluster to
> run the CLI against. Future unit tests will use this infrastructure to
> test their commands against a running mesos cluster. The tests require
> some binaries created when building Mesos.
> 
> 
> Diffs
> -
> 
>   src/cli_new/lib/cli/tests/__init__.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/base.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/constants.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/tests.py PRE-CREATION 
>   src/cli_new/pip-requirements.txt 28613e56a5c6d5c7606a7e58d6125b0c34748e83 
>   src/cli_new/tests/main.py dff5d48b0ddae87960a78f9d05e4ae597912f1f6 
> 
> 
> Diff: https://reviews.apache.org/r/58720/diff/14/
> 
> 
> Testing
> ---
> 
> PEP8 and Pylint used to make sure that the code style is correct. Manuel test:
> 
> $ cd src/cli_new
> $ ./bootstrap
> $ source activate
> 
> (mesos-cli) $ mesos-cli-tests
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 58720: Extended the unit test infrastructure in the new Mesos CLI.

[Armand Grillet]

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

(Updated May 30, 2017, 10 a.m.)


Review request for mesos and Kevin Klues.


Changes
---

Code rebased and issues regarding imports and `systemd_enable_support` fixed.


Bugs: MESOS-7283
https://issues.apache.org/jira/browse/MESOS-7283


Repository: mesos


Description
---

This infrastructure includes the ability to bring up a test cluster to
run the CLI against. Future unit tests will use this infrastructure to
test their commands against a running mesos cluster. The tests require
some binaries created when building Mesos.


Diffs (updated)
-

  src/cli_new/lib/cli/tests/__init__.py PRE-CREATION 
  src/cli_new/lib/cli/tests/base.py PRE-CREATION 
  src/cli_new/lib/cli/tests/constants.py PRE-CREATION 
  src/cli_new/lib/cli/tests/tests.py PRE-CREATION 
  src/cli_new/pip-requirements.txt 28613e56a5c6d5c7606a7e58d6125b0c34748e83 
  src/cli_new/tests/main.py dff5d48b0ddae87960a78f9d05e4ae597912f1f6 


Diff: https://reviews.apache.org/r/58720/diff/14/

Changes: https://reviews.apache.org/r/58720/diff/13-14/


Testing
---

PEP8 and Pylint used to make sure that the code style is correct. Manuel test:

$ cd src/cli_new
$ ./bootstrap
$ source activate

(mesos-cli) $ mesos-cli-tests


Thanks,

Armand Grillet

Re: Review Request 58720: Extended the unit test infrastructure in the new Mesos CLI.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [59628, 58720]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker-build.sh

- Mesos Reviewbot


On May 29, 2017, 5:27 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58720/
> ---
> 
> (Updated May 29, 2017, 5:27 p.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-7283
> https://issues.apache.org/jira/browse/MESOS-7283
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This infrastructure includes the ability to bring up a test cluster to
> run the CLI against. Future unit tests will use this infrastructure to
> test their commands against a running mesos cluster. The tests require
> some binaries created when building Mesos.
> 
> 
> Diffs
> -
> 
>   src/cli_new/lib/cli/__init__.py 4ddbb0eb5ea2c79db852e7b27ef702869316c3f3 
>   src/cli_new/lib/cli/tests/__init__.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/base.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/constants.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/tests.py PRE-CREATION 
>   src/cli_new/pip-requirements.txt 28613e56a5c6d5c7606a7e58d6125b0c34748e83 
>   src/cli_new/tests/main.py dff5d48b0ddae87960a78f9d05e4ae597912f1f6 
> 
> 
> Diff: https://reviews.apache.org/r/58720/diff/13/
> 
> 
> Testing
> ---
> 
> PEP8 and Pylint used to make sure that the code style is correct. Manuel test:
> 
> $ cd src/cli_new
> $ ./bootstrap
> $ source activate
> 
> (mesos-cli) $ mesos-cli-tests
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 58720: Extended the unit test infrastructure in the new Mesos CLI.

[Kevin Klues]

> On May 23, 2017, 4:36 p.m., Andrei Budnik wrote:
> > src/cli_new/lib/cli/tests/base.py
> > Lines 321 (patched)
> > 
> >
> > return any(container["executor_id"] == self.flags["name"] for container 
> > in data)
> > 
> > BTW, should we use lambda function here like I've mentioned above?

I don't think it should become a lambda (for consistency), but I do agree you 
could write it in Andrei's more concise form:


- Kevin


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


On May 29, 2017, 3:27 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58720/
> ---
> 
> (Updated May 29, 2017, 3:27 p.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-7283
> https://issues.apache.org/jira/browse/MESOS-7283
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This infrastructure includes the ability to bring up a test cluster to
> run the CLI against. Future unit tests will use this infrastructure to
> test their commands against a running mesos cluster. The tests require
> some binaries created when building Mesos.
> 
> 
> Diffs
> -
> 
>   src/cli_new/lib/cli/__init__.py 4ddbb0eb5ea2c79db852e7b27ef702869316c3f3 
>   src/cli_new/lib/cli/tests/__init__.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/base.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/constants.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/tests.py PRE-CREATION 
>   src/cli_new/pip-requirements.txt 28613e56a5c6d5c7606a7e58d6125b0c34748e83 
>   src/cli_new/tests/main.py dff5d48b0ddae87960a78f9d05e4ae597912f1f6 
> 
> 
> Diff: https://reviews.apache.org/r/58720/diff/13/
> 
> 
> Testing
> ---
> 
> PEP8 and Pylint used to make sure that the code style is correct. Manuel test:
> 
> $ cd src/cli_new
> $ ./bootstrap
> $ source activate
> 
> (mesos-cli) $ mesos-cli-tests
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 58720: Extended the unit test infrastructure in the new Mesos CLI.

[Kevin Klues]

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




src/cli_new/lib/cli/tests/base.py
Lines 177-188 (patched)


I had to add the following to get this to run on my system without sudo:
```
if "systemd_enable_support" not in flags:
flags["systemd_enable_support"] = "false"
```

This seems like a reasonable default to set...


- Kevin Klues


On May 29, 2017, 3:27 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58720/
> ---
> 
> (Updated May 29, 2017, 3:27 p.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-7283
> https://issues.apache.org/jira/browse/MESOS-7283
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This infrastructure includes the ability to bring up a test cluster to
> run the CLI against. Future unit tests will use this infrastructure to
> test their commands against a running mesos cluster. The tests require
> some binaries created when building Mesos.
> 
> 
> Diffs
> -
> 
>   src/cli_new/lib/cli/__init__.py 4ddbb0eb5ea2c79db852e7b27ef702869316c3f3 
>   src/cli_new/lib/cli/tests/__init__.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/base.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/constants.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/tests.py PRE-CREATION 
>   src/cli_new/pip-requirements.txt 28613e56a5c6d5c7606a7e58d6125b0c34748e83 
>   src/cli_new/tests/main.py dff5d48b0ddae87960a78f9d05e4ae597912f1f6 
> 
> 
> Diff: https://reviews.apache.org/r/58720/diff/13/
> 
> 
> Testing
> ---
> 
> PEP8 and Pylint used to make sure that the code style is correct. Manuel test:
> 
> $ cd src/cli_new
> $ ./bootstrap
> $ source activate
> 
> (mesos-cli) $ mesos-cli-tests
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 58720: Extended the unit test infrastructure in the new Mesos CLI.

[Kevin Klues]

> On May 7, 2017, 8:13 p.m., Kevin Klues wrote:
> > src/cli_new/lib/cli/__init__.py
> > Line 18 (original), 18 (patched)
> > 
> >
> > This should be called Mesos CLI module since the renaming. This 
> > shouldn't be bundled into this patch though. It should be it's own isolated 
> > one.

I fixed this in a one-off commit and pushed it.


> On May 7, 2017, 8:13 p.m., Kevin Klues wrote:
> > src/cli_new/lib/cli/tests/base.py
> > Lines 384-401 (patched)
> > 
> >
> > This is fine for now, but I'm wondering if we can't find a more generic 
> > way of discovering the build directorty.  This current method assumes we 
> > always create the build directory in a specific location.
> > 
> > It also assumes we have a build directory at all. Which maybe we should 
> > safeguard againts.
> 
> Armand Grillet wrote:
> True, we could add an option like for our configure script with 
> `--with-mesos-build-dir`. Our script mesos-cli-tests is currently small and 
> supporting long command line options is gonna require a lot of code (it does 
> not work out of the box on macOS). The easiest solution would be to have a 
> short command line option to specify your build path (e.g. `mesos-cli-tests 
> -b=path/to/mesos/build/dir`).

Since this sounds like a configuration time thing, I'd rather not have this 
function living in this `tests.py` file. We should let it be set to the default 
as you have, but also allow it to be passed in from above somehow.

I'm thinking, we could move it into `CLITestCase` as a static method and rename 
it to `default_mesos_build_directory()`. We can then have a class variable 
called `MESOS_BUILD_DIRECTORY` which is initially set to 
`default_mesos_build_directory()`, but can be overriden (if desired) in 
`src/cli_new/tests/main.py`.

Something like:
```
class CLITestCase(unittest.TestCase):
"""
Base class for CLI TestCases.
"""
@classmethod
def setUpClass(cls):
print "\n{class_name}".format(class_name=cls.__name__)

@staticmethod
def default_mesos_build_directory():
"""
Returns the default path of the Mesos build directory. Useful when
we wish to use some binaries such as mesos-execute.
"""
tests_dir = os.path.dirname(__file__)
cli_dir = os.path.dirname(tests_dir)
lib_dir = os.path.dirname(cli_dir)
cli_new_dir = os.path.dirname(lib_dir)
src_dir = os.path.dirname(cli_new_dir)
mesos_dir = os.path.dirname(src_dir)
build_dir = os.path.join(mesos_dir, "build")

if os.path.isdir(build_dir):
return build_dir
else:
raise CLIException("The Mesos build directory does not exist: 
{path}"
   .format(path=build_dir))

CLITestCase.MESOS_BUILD_DIRECTORY = CLITestCase.default_mesos_build_directory()
```

And then if we ever wanted to override it we could do (for example):
```
# pylint: disable=unused-import
# We import the tests that we want to run.
from cli.tests import CLITestCase
from cli.tests import TestInfrastructure

if __name__ == '__main__':
CLITestCase.MESOS_BUILD_DIRECTORY = ...

print colored("Running the Mesos CLI unit tests", "yellow")
unittest.main(verbosity=2, testRunner=unittest.TextTestRunner)
```


- Kevin


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


On May 29, 2017, 3:27 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58720/
> ---
> 
> (Updated May 29, 2017, 3:27 p.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-7283
> https://issues.apache.org/jira/browse/MESOS-7283
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This infrastructure includes the ability to bring up a test cluster to
> run the CLI against. Future unit tests will use this infrastructure to
> test their commands against a running mesos cluster. The tests require
> some binaries created when building Mesos.
> 
> 
> Diffs
> -
> 
>   src/cli_new/lib/cli/__init__.py 4ddbb0eb5ea2c79db852e7b27ef702869316c3f3 
>   src/cli_new/lib/cli/tests/__init__.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/base.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/constants.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/tests.py PRE-CREATION 
>   src/cli_new/pip-requirements.txt 28613e56a5c6d5c7606a7e58d6125b0c34748e83 
>   src/cli_new/tests/main.py dff5d48b0ddae87960a78f9d05e4ae597912f1f6 
> 
> 
> Diff: https://reviews.apache.org/r/58720/diff/13/
> 
> 

Re: Review Request 58720: Extended the unit test infrastructure in the new Mesos CLI.

[Kevin Klues]

> On May 29, 2017, 10:24 p.m., Kevin Klues wrote:
> > This is still failing for me after the rebase:
> > ```
> > (mesos-cli) $ mesos
> > Traceback (most recent call last):
> >   File "/scratch/klueska/projects/mesos/src/cli_new/bin/main.py", line 25, 
> > in 
> > import cli
> >   File "/scratch/klueska/projects/mesos/src/cli_new/lib/cli/__init__.py", 
> > line 22, in 
> > from . import constants
> > ImportError: cannot import name constants
> > ```
> > 
> > Then after fixing this error by removing `from . import constants` from 
> > `__init__.py`:
> > 
> > ```
> > (mesos-cli) $ mesos-cli-tests
> > Traceback (most recent call last):
> >   File "/scratch/klueska/projects/mesos/src/cli_new/bin/../tests/main.py", 
> > line 27, in 
> > from cli.tests import TestInfrastructure
> >   File "/scratch/klueska/projects/mesos/src/cli_new/lib/cli/__init__.py", 
> > line 21, in 
> > from . import config
> >   File "/scratch/klueska/projects/mesos/src/cli_new/lib/cli/config.py", 
> > line 24, in 
> > import settings
> > ImportError: No module named settings
> > ```

I see what happened, I didn't apply https://reviews.apache.org/r/59628/ before 
running this. Please see comments there.


- Kevin


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


On May 29, 2017, 3:27 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58720/
> ---
> 
> (Updated May 29, 2017, 3:27 p.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-7283
> https://issues.apache.org/jira/browse/MESOS-7283
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This infrastructure includes the ability to bring up a test cluster to
> run the CLI against. Future unit tests will use this infrastructure to
> test their commands against a running mesos cluster. The tests require
> some binaries created when building Mesos.
> 
> 
> Diffs
> -
> 
>   src/cli_new/lib/cli/__init__.py 4ddbb0eb5ea2c79db852e7b27ef702869316c3f3 
>   src/cli_new/lib/cli/tests/__init__.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/base.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/constants.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/tests.py PRE-CREATION 
>   src/cli_new/pip-requirements.txt 28613e56a5c6d5c7606a7e58d6125b0c34748e83 
>   src/cli_new/tests/main.py dff5d48b0ddae87960a78f9d05e4ae597912f1f6 
> 
> 
> Diff: https://reviews.apache.org/r/58720/diff/13/
> 
> 
> Testing
> ---
> 
> PEP8 and Pylint used to make sure that the code style is correct. Manuel test:
> 
> $ cd src/cli_new
> $ ./bootstrap
> $ source activate
> 
> (mesos-cli) $ mesos-cli-tests
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 58720: Extended the unit test infrastructure in the new Mesos CLI.

[Kevin Klues]

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



This is still failing for me after the rebase:
```
(mesos-cli) $ mesos
Traceback (most recent call last):
  File "/scratch/klueska/projects/mesos/src/cli_new/bin/main.py", line 25, in 

import cli
  File "/scratch/klueska/projects/mesos/src/cli_new/lib/cli/__init__.py", line 
22, in 
from . import constants
ImportError: cannot import name constants
```

Then after fixing this error by removing `from . import constants` from 
`__init__.py`:

```
(mesos-cli) $ mesos-cli-tests
Traceback (most recent call last):
  File "/scratch/klueska/projects/mesos/src/cli_new/bin/../tests/main.py", line 
27, in 
from cli.tests import TestInfrastructure
  File "/scratch/klueska/projects/mesos/src/cli_new/lib/cli/__init__.py", line 
21, in 
from . import config
  File "/scratch/klueska/projects/mesos/src/cli_new/lib/cli/config.py", line 
24, in 
import settings
ImportError: No module named settings
```


src/cli_new/lib/cli/__init__.py
Lines 22 (patched)


This file doesn't exist, so we can't import it.


- Kevin Klues


On May 29, 2017, 3:27 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58720/
> ---
> 
> (Updated May 29, 2017, 3:27 p.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-7283
> https://issues.apache.org/jira/browse/MESOS-7283
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This infrastructure includes the ability to bring up a test cluster to
> run the CLI against. Future unit tests will use this infrastructure to
> test their commands against a running mesos cluster. The tests require
> some binaries created when building Mesos.
> 
> 
> Diffs
> -
> 
>   src/cli_new/lib/cli/__init__.py 4ddbb0eb5ea2c79db852e7b27ef702869316c3f3 
>   src/cli_new/lib/cli/tests/__init__.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/base.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/constants.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/tests.py PRE-CREATION 
>   src/cli_new/pip-requirements.txt 28613e56a5c6d5c7606a7e58d6125b0c34748e83 
>   src/cli_new/tests/main.py dff5d48b0ddae87960a78f9d05e4ae597912f1f6 
> 
> 
> Diff: https://reviews.apache.org/r/58720/diff/13/
> 
> 
> Testing
> ---
> 
> PEP8 and Pylint used to make sure that the code style is correct. Manuel test:
> 
> $ cd src/cli_new
> $ ./bootstrap
> $ source activate
> 
> (mesos-cli) $ mesos-cli-tests
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 58720: Extended the unit test infrastructure in the new Mesos CLI.

[Kevin Klues]

> On May 23, 2017, 6:30 p.m., Andrei Budnik wrote:
> > src/cli_new/lib/cli/tests/base.py
> > Lines 265 (patched)
> > 
> >
> > Is it possible to run multiple tasks? What will be in case of task-id 
> > collision?
> > 
> > It's possible to omit str() in '.format(id=str(random.random()))'. 
> > Also, random.random() returns floating point number in the range [0.0, 1.0).

Good catch!


- Kevin


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


On May 29, 2017, 3:27 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58720/
> ---
> 
> (Updated May 29, 2017, 3:27 p.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-7283
> https://issues.apache.org/jira/browse/MESOS-7283
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This infrastructure includes the ability to bring up a test cluster to
> run the CLI against. Future unit tests will use this infrastructure to
> test their commands against a running mesos cluster. The tests require
> some binaries created when building Mesos.
> 
> 
> Diffs
> -
> 
>   src/cli_new/lib/cli/__init__.py 4ddbb0eb5ea2c79db852e7b27ef702869316c3f3 
>   src/cli_new/lib/cli/tests/__init__.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/base.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/constants.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/tests.py PRE-CREATION 
>   src/cli_new/pip-requirements.txt 28613e56a5c6d5c7606a7e58d6125b0c34748e83 
>   src/cli_new/tests/main.py dff5d48b0ddae87960a78f9d05e4ae597912f1f6 
> 
> 
> Diff: https://reviews.apache.org/r/58720/diff/13/
> 
> 
> Testing
> ---
> 
> PEP8 and Pylint used to make sure that the code style is correct. Manuel test:
> 
> $ cd src/cli_new
> $ ./bootstrap
> $ source activate
> 
> (mesos-cli) $ mesos-cli-tests
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 58720: Extended the unit test infrastructure in the new Mesos CLI.

[Kevin Klues]

> On May 24, 2017, 2:03 p.m., Andrei Budnik wrote:
> > src/cli_new/lib/cli/tests/base.py
> > Lines 269 (patched)
> > 
> >
> > Please, use Task.task_id instead of self.task_id:
> > flags["name"] = "task-{id}".format(id=Task.task_id)
> > Task.task_id += 1
> > 
> > Otherwise every instance of Task(Executable) will have same task_id.

Good catch!


- Kevin


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


On May 29, 2017, 3:27 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58720/
> ---
> 
> (Updated May 29, 2017, 3:27 p.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-7283
> https://issues.apache.org/jira/browse/MESOS-7283
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This infrastructure includes the ability to bring up a test cluster to
> run the CLI against. Future unit tests will use this infrastructure to
> test their commands against a running mesos cluster. The tests require
> some binaries created when building Mesos.
> 
> 
> Diffs
> -
> 
>   src/cli_new/lib/cli/__init__.py 4ddbb0eb5ea2c79db852e7b27ef702869316c3f3 
>   src/cli_new/lib/cli/tests/__init__.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/base.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/constants.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/tests.py PRE-CREATION 
>   src/cli_new/pip-requirements.txt 28613e56a5c6d5c7606a7e58d6125b0c34748e83 
>   src/cli_new/tests/main.py dff5d48b0ddae87960a78f9d05e4ae597912f1f6 
> 
> 
> Diff: https://reviews.apache.org/r/58720/diff/13/
> 
> 
> Testing
> ---
> 
> PEP8 and Pylint used to make sure that the code style is correct. Manuel test:
> 
> $ cd src/cli_new
> $ ./bootstrap
> $ source activate
> 
> (mesos-cli) $ mesos-cli-tests
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 58720: Extended the unit test infrastructure in the new Mesos CLI.

[Kevin Klues]

> On May 23, 2017, 3:59 p.m., Andrei Budnik wrote:
> > src/cli_new/lib/cli/tests/base.py
> > Lines 52 (patched)
> > 
> >
> > I would prefer os.linesep to '\n' here and everywhere to avoid extra 
> > constant.

The python documentation disagrees:

https://docs.python.org/3.0/library/os.html

```
os.linesep

The string used to separate (or, rather, terminate) lines on the current 
platform. This may be a single character, such as '\n' for POSIX, or multiple 
characters, for example, '\r\n' for Windows. Do not use os.linesep as a line 
terminator when writing files opened in text mode (the default); use a single 
'\n' instead, on all platforms.
```


- Kevin


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


On May 29, 2017, 3:27 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58720/
> ---
> 
> (Updated May 29, 2017, 3:27 p.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-7283
> https://issues.apache.org/jira/browse/MESOS-7283
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This infrastructure includes the ability to bring up a test cluster to
> run the CLI against. Future unit tests will use this infrastructure to
> test their commands against a running mesos cluster. The tests require
> some binaries created when building Mesos.
> 
> 
> Diffs
> -
> 
>   src/cli_new/lib/cli/__init__.py 4ddbb0eb5ea2c79db852e7b27ef702869316c3f3 
>   src/cli_new/lib/cli/tests/__init__.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/base.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/constants.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/tests.py PRE-CREATION 
>   src/cli_new/pip-requirements.txt 28613e56a5c6d5c7606a7e58d6125b0c34748e83 
>   src/cli_new/tests/main.py dff5d48b0ddae87960a78f9d05e4ae597912f1f6 
> 
> 
> Diff: https://reviews.apache.org/r/58720/diff/13/
> 
> 
> Testing
> ---
> 
> PEP8 and Pylint used to make sure that the code style is correct. Manuel test:
> 
> $ cd src/cli_new
> $ ./bootstrap
> $ source activate
> 
> (mesos-cli) $ mesos-cli-tests
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 58720: Extended the unit test infrastructure in the new Mesos CLI.

[Armand Grillet]

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

(Updated May 29, 2017, 3:27 p.m.)


Review request for mesos and Kevin Klues.


Changes
---

Code rebased.


Summary (updated)
-

Extended the unit test infrastructure in the new Mesos CLI.


Bugs: MESOS-7283
https://issues.apache.org/jira/browse/MESOS-7283


Repository: mesos


Description
---

This infrastructure includes the ability to bring up a test cluster to
run the CLI against. Future unit tests will use this infrastructure to
test their commands against a running mesos cluster. The tests require
some binaries created when building Mesos.


Diffs (updated)
-

  src/cli_new/lib/cli/__init__.py 4ddbb0eb5ea2c79db852e7b27ef702869316c3f3 
  src/cli_new/lib/cli/tests/__init__.py PRE-CREATION 
  src/cli_new/lib/cli/tests/base.py PRE-CREATION 
  src/cli_new/lib/cli/tests/constants.py PRE-CREATION 
  src/cli_new/lib/cli/tests/tests.py PRE-CREATION 
  src/cli_new/pip-requirements.txt 28613e56a5c6d5c7606a7e58d6125b0c34748e83 
  src/cli_new/tests/main.py dff5d48b0ddae87960a78f9d05e4ae597912f1f6 


Diff: https://reviews.apache.org/r/58720/diff/13/

Changes: https://reviews.apache.org/r/58720/diff/12-13/


Testing
---

PEP8 and Pylint used to make sure that the code style is correct. Manuel test:

$ cd src/cli_new
$ ./bootstrap
$ source activate

(mesos-cli) $ mesos-cli-tests


Thanks,

Armand Grillet