Run installable tests

[dHannasch]

Minor suggestion for pytest default settings --- going for least astonishment, someone who puts a test_* file in src/ (that is, a test to be installed with the package) probably will not be surprised or confused if pytest finds and runs those tests.

[ionelmc]

Unfortunately this will break everything in practice because there would be two unpredictable sources for code - src and the stuff in site-packages.

For people that need to collect test in src there is the test_matrix_separate_coverage option.

[ionelmc]

Or without test_matrix_separate_coverage you'd basically need to have testpaths = tests path/to/site-packages/mypackage which is not adequate from the get-go.

[dHannasch]

there would be two unpredictable sources for code - src and the stuff in site-packages.

If I'm understanding you correctly, the problem is that after making a point of isolating code in src/ so that we're forced to test the installed version of the code (https://blog.ionelmc.ro/2014/05/25/python-packaging/#the-structure), we're not testing the installed version of the tests, correct?

That's a good point. What if we used --pyargs to run the installed version of the tests?

addopts =
    --pyargs
testpaths =
    {{cookiecutter.package_name}}

I realize it looks strange, but it appears this really is pytest's intended interaction between pyargs and testpaths, and it got fixed when pytest 4.0.0 briefly broke it: pytest-dev/pytest#4405

Someone suggested allowing package names to be specified in testpaths itself (pytest-dev/pytest#3405 (comment)), but that never got off the ground since technically that behavior is already possible in pytest (albeit strange-looking).

[ionelmc]

Hmmm very interesting (and what a horrible choice for option name). We could have a "tests_inside_package" or "package_contains_test" if you fancy the idea of waddling around in the templates again :)

[ionelmc]

I mean, in the idea that adding --pyargs will mess up usage of "tests outside" and it's not a good default (I guess there would be some surprise factor when playing with extra collection options, and tests that do crazy stuff at import time blowing up when tools try to inspect installed modules).

[dHannasch]

We could have a "tests_inside_package" or "package_contains_test"
adding --pyargs will mess up usage of "tests outside" and it's not a good default (I guess there would be some surprise factor when playing with extra collection options, and tests that do crazy stuff at import time blowing up when tools try to inspect installed modules).

I have no objection to making a run_installable_tests option per se, but I'm a bit confused by your comment.

I'm not sure what you mean by mess up usage of tests outside? This still runs the non-installed tests as normal: https://github.com/ionelmc/cookiecutter-pylibrary/pull/123/files#diff-9187e1a09582306a1c23d6b37065dffdR52

I put a / at the end so even if somebody has installed a package literally named tests (https://pypi.org/project/tests/), pytest will still run the tests in the ./tests/ directory because it can't parse "tests/" as a package name.

(Though now that I think about it, I'll have to check whether that works on Windows...sometimes Python tools will accept / paths on Windows pallets/jinja#411 (comment) , but I don't know about pytset testpaths. Might need something to put a \ there on Windows. Looks like there are ways, and probably / is fine in any event: pallets/jinja#367)

I don't see any way that this could interfere with running the "outside" non-installed tests. Is there a possibility I'm overlooking?

We're definitely going to have to import the installed {{cookiecutter.package_name}} to run any tests at all, including the ones in tests/ --- indeed we went out of our way to make sure we were forced to import the installed package when running the tests, because we specifically wanted that (https://blog.ionelmc.ro/2014/05/25/python-packaging/#the-structure).

I would be very surprised if anyone was surprised by the installed tests being run, considering that in order for any installed tests to get run, the user would first have to write those tests. Someone who sits down to write a test presumably intends to run it, I would think. Even if --- especially if --- they're making that test available for their own users to run with pytest --pyargs {{cookiecutter.package_name}}.

Does anything unusual happen when tools try to inspect installed modules? I mean...I'm not an expert on Python packaging, but I thought an installed package was literally just a folder.

My biggest concern is people playing with extra collection options you mentioned --- I'm not very familiar with all those. I wouldn't think that any of the test-collection options pytest has would do anything crazy like "escape" the package folder and go running around elsewhere in site-packages, but it sounds like you'd know more about that than I would. Are there any pytest collection options you're thinking of that would go and do crazy things if we were running tests out of an installed package?

[ionelmc]

I guess this is relevant: https://github.com/pytest-dev/pytest/issues?utf8=%E2%9C%93&q=is%3Aissue+is%3Aopen+pyargs and I suspect there are more unknown ones because few people use the obscure --pyargs.

It's basically gut feeling + the fact that other "late and not really wanted features" of pytest like unittest support have various problems and are not orthogonal to other pytest features (won't work well or integrate well with other features or CLI options).

Regardless of technical implications, can't just switch test layout overnight. Maybe later we can change the default - if it work well.

You agree on having an option for this?

[ionelmc]

Just one small nit, in the vein of the discussion on the other PR - the option prefixing (I would like to have the option start with "test") :)

Some suggestions:

• tests_installed_with_package
• tests_inside_package
• tests_in_package
• tests_installable
• test_installed_package
• test_without_sources
• test_without_repo

[dHannasch]

I guess this is relevant: https://github.com/pytest-dev/pytest/issues?utf8=%E2%9C%93&q=is%3Aissue+is%3Aopen+pyargs and I suspect there are more unknown ones because few people use the obscure --pyargs.

Yikes. That's a lot. Since pyargs has been around so long, I always sort of assumed it was just poorly documented, not buggy.

What do you think of the name tests_installed_with_package_also_run?

Hmm. The command "openssl aes-256-cbc -K $encrypted_a70d5afca909_key -iv $encrypted_a70d5afca909_iv -in publish-key.enc -out ~/.ssh/publish-key -d" exited with 1. Probably related to 7f5dd2c and the bug you were trying to fix with 5de08d3 . Unfortunately it's not at all clear why openssl would crash.

[ionelmc]

Not sure what to say about the name. I guess I need to find some time to really try --pyargs and see how it behaves in practice, and also overhaul the CI a bit, sigh ...

[dHannasch]

For what it's worth, the autodeploy looks incredibly cool (to the extent I understand it, which isn't much). No idea why it isn't working.

pytest --pyargs {{name}} is a lifesaver if you do a lot of subprocess shenanigans or low-level hacking that depends on the host machine in unpredictable ways. It'll always find the tests no matter what variety of Python the user has installed or what kind of virtual environment they're using, and the full work-up is usually enough for a good swift guess as to what's different about the environment that's throwing a wrench in matters. (Of course, coming up with a patch to allow the package to work in that environment isn't any easier, but at least you don't have to fiddle around wondering what the problem is.)
If you don't do any such shenanigans, then you can easily go your entire life without wanting or needing pyargs, which is probably why it gets less care.

[ionelmc]

@dHannasch so I've played a bit with pyargs, and I think to avoid confusion this option should be conflated with the move of the tests inside the package (eg, have os.rename('tests', 'src/{pakcage_name}/tests'); create 'src/{pakcage_name}/tests/__init__.py inside the post gen hook).

You still wanna work on this?

[dHannasch]

I think to avoid confusion this option should be conflated with the move of the tests inside the package (eg, have os.rename('tests', 'src/{pakcage_name}/tests'); create 'src/{pakcage_name}/tests/init.py inside the post gen hook)

Sorry, I don't understand. Are you saying that a package should only ever have tests-that-are-installed-with-the-package or tests-that-are-not-installed-with-the-package, never both?

I would assume that the tests directory outside of src/ would still be there as normal.

As for creating src/tests/__init__.py, sure, I guess if you want, it's no trouble. Personally I'm a pytest user, so my test files always start with test_ anyway, so I never thought of making a tests namespace to hold them. But if you like the idea of a tests namespace, I certainly can't think of any reason not to have one.

But I don't really see why we would want to delete the non-installed tests/ directory. Any use-cases for that, such as needing large files for some tests (and not wanting to include those large files with the package when it's installed), would seem to still apply, even if some other tests get installed with the package.

What is the confusion that you want to avoid?

[ionelmc]

Mainly the confusion around more lenient test collection. Users won't get any errors if argument is not a path but some package name.

The other reason is that my aim with this template is to produce "hand-written"-like output. No one would use --pyargs if they don't have tests inside package, thus this template should produce that sort of output.

And it's not src/tests/something but src/package_name/tests/something.

To put this differently, this template should not produce something made halfway (I think making users do more changes to make tests inside package work, like adding collection arguments in various places) but produce something that works end to end - namely a working project with tests inside package - as I understand that's the underlying goal here.

If someone needs huge test files then they should not use this option. This should be made note of somewhere in option description.

[dHannasch]

Well, I made tests/__init__.py like you said, and set it up to get deleted if the user says no to installable tests. I used shutil.rmtree like you did when they say no to docs. That part, at least, is what you have in mind, right?

And it's not src/tests/something but src/package_name/tests/something.

Ah, oops. Well, I did get it right in the actual commit, right? b93e5a7

I think there's another change you also want, but I'm not understanding what it is.

The other reason is that my aim with this template is to produce "hand-written"-like output. No one would use --pyargs if they don't have tests inside package, thus this template should produce that sort of output.

As far as I'm aware, that's the way it's already set up. If cookiecutter.tests_installed_with_package_also_run == 'yes', then --pyargs appears in setup.cfg. If the user said no to installed tests, then --pyargs doesn't appear anywhere.
Is there a way the jinja2 template could work that I'm missing?

this template should not produce something made halfway (I think making users do more changes to make tests inside package work, like adding collection arguments in various places) but produce something that works end to end - namely a working project with tests inside package - as I understand that's the underlying goal here.

I agree with everything you're saying here, but I have no idea what it has to do with anything we're talking about. What part is "halfway"?

If someone needs huge test files then they should not use this option.

Consider a package for astronomical scientific computing called yt. yt has a tests subpackage (with __init__.py) inside the package. Those tests are installed with the package and can be run by the user. yt also has some large, elaborate tests that require astronomical datasets; those tests should not be installed with the package, because the datasets are large. Thus, yt has two tests directories, one that is installed with the package, one that is not installed with the package.

Users won't get any errors if argument is not a path but some package name.

Do you mean that if the user makes a typo when typing a path to a specific test they want to run, and by coincidence the typo makes it look exactly like the name of an unrelated Python package that they happen to have installed in their current environment, then instead of getting an error that the path does not exist, they'll be surprised when pytest goes and runs the installed tests for that unrelated package?

That would certainly be bad. And if that were very likely to happen, we might want to explore alternatives, like directly specifying src/packageName/tests/ as a test directory instead of using --pyargs, even though that would mean running the installed version of the code on a potentially-different version of the test code. (Which...I don't think is what you're getting at, in any event, but I'm not sure.)

But that typo seems incredibly unlikely. I mean, any test path is going to look like tests/something or src/packageName/tests/something. Python package names can't have slashes in them. Imagine what it would take for someone to set out to type pytest src/packageName/tests/test_re.py and end up with pytest re. That's not literally impossible, but that's one heck of a typo. Am I missing some possibility here?

(And, I mean, if that unlikely event does occur, which it inevitably will eventually...the consequences don't exactly seem dire. The installed tests of the package re, if any, will get run. The user will be briefly confused and then realize they made a typo.)

[ionelmc]

Oook ... fine. About the option name, you like any of these alternatives?

• collect_tests_from_package
• allow_tests_inside_package

[dHannasch]

collect_tests_from_package sounds fine. I started out calling it run_tests_that_are_installed_with_package, which is the same thing, just longer and (I thought) more clear. The reason I changed to tests_installed_with_package_also_run 45acd63 was because I thought you wanted it to start with tests_ . #123 (comment)

(I assumed because the options are sometimes sorted alphabetically? I've never observed that myself, but if the options might get shuffled into alphabetical order, then we certainly want to keep test-related options together. But I've never seen them get sorted alphabetically, so I'm fine with collect_tests_from_package if you are.)