> On May 7, 2017, 8:13 p.m., Kevin Klues wrote: > > src/cli_new/lib/cli/__init__.py > > Line 18 (original), 18 (patched) > > <https://reviews.apache.org/r/58720/diff/4/?file=1709418#file1709418line18> > > > > 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) > > <https://reviews.apache.org/r/58720/diff/4/?file=1709421#file1709421line384> > > > > 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/ > > > 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 > >
