Hi, Answers inline below, It's unclear from the nose source[1] whether it's calling build_py >> and build_ext, or just build_ext. It's also unclear whether the result of >> that build is actually used. When python setup.py nosetests runs, it runs >> inside of a virtualenv created by tox, and tox has already installed beam >> into that venv. It seems unlikely to me that build_ext or build_py is >> going to install over top of the beam package installed by tox, but who >> knows, it may end up first on the path. Also recall that there is an sdist >> gradle task running before tox that creates a tarball that is passed to >> run_tox.sh which passes it along to tox --installpkg flag[2] so that tox >> won't build beam itself. >> > > I believe the build step is executed as expected and during installation > results in cythonized package to be installed. This could be verified by, > in a new virtual environment creating a source distribution, installing > cython, then installing the source distribution. Resulting installation > does have the .so files. This is done before running nosetests. >
Even if it *is* working, I think it's a pretty poor design that we build it once in sdist and then rebuild it again with nose. It's very obfuscated and brittle, hence we're still debating the probable outcome. We should choose one place to build and that should either be the sdist gradle task or tox, not the test command. > We should designate a single place that always does the build. I thought >> that was supposed to be the gradle sdist task, but invoking nose via >> `python setup.py` means that we're introducing the possibility that a build >> is occurring which may or may not be used, depending on the entirely >> unclear dependencies of the setup commands, and the entirely unclear >> relationship of that build output to the tox venv. As a first step of >> clarity, we could stop invoking nose using `python setup.py nosetests`, and >> instead use `nosetests` (and in the future `pytest`). I think the reason >> for `python setup.py nosetest` is to ensure the test requirements are >> installed, >> > > I believe the reason we are invokign nosetest this way is related to the > beam testing plugin. It is configure in setup.py. The behavior is > documented here: https://nose.readthedocs.io/en/latest/api/commands.html > It is possible to register a custom plugin without using setup.py: https://nose.readthedocs.io/en/latest/plugins/writing.html#registering-a-plugin-without-setuptools Since we're on the verge of switching to pytest, perhaps we should investigate switching that over to to not use setup.py instead of chasing our tails with nose. > but we could shift those to a separate requirements file or use >> extra_requires and tox can ensure that they're installed. I find these two >> options to be pretty common patterns [3]. >> > > We do use extras is tox already. GCP tests work this way by installing > additional GCP package. In my opinion, letting tox to setup the virtual > environment either from the package or from setup.py is a better option > than using requirements file. Otherwise we would need a way to keep > setup.py and requirements file in sync. > Oh yeah, I see that the tests already are an extra package. Well, that'll make it that much easier to stop using `python setup.py nosetests`. -chad
