> 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`). > > 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/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 > >
