Pytest test structure?

Plone Support Add-on Development
1

Hi folks,

there are already a couple of addon's using Pytest for testing.
Like GitHub - collective/collective.blog: Blog content types for Plone 6 for example.
The test structure differs from what we are normally using in Plone.
That means that test are placed on the root level of the package and not close to the code. I know that this is recommended by some Python folks out there. But i heart also other opinions about this.
Is this the way we want to go for Plone?
It's fine with me, i just don't want to start this way, when we start a discussion later with a different outcome.

Also I'm i right, that we have to port all test of a package to pytest at once or is there a way that our setup can be migrated test by test?
Pytest seams to support this in general, but first test failed for me, could be test layer thing maybe?

2

Tests should not be in a dist. They are only used by developers of and contributors to the package. Same for docs. The best practice in Python is exemplified in Pyramid.

.
├── docs
├── src/pyramid
└── tests
2 Likes
3

I would like to not shutdown the discussion by "should not do" arguments. Let's please just focus on what the benefit going one or the other way. We know where we coming from and what others do.
I'm pretty sure there good reasons on either side.
Let's discuss this in a friendly tone and let's see to what we consensus we are coming.

1 Like
4

Regarding excluding tests from the dist, this would be possible via manifest file too. This way they would be still close to the code, which is one argument i know of to put them there.

5

@davisagli @jensens @1letter @petschki @pbauer @mauritsvanrees @erral @ericof opinions?

6

I prefer pytest, but i need a working setup for e2e tests.
Running robot tests/playwright test in a coverage pytest command or whatever.

7

the question is not, pytest or not, more about where we put the tests when we use pytest?

8

If i see the structure in addons, then the test folder in the root path of package is the preferred place. For me is the place irrelevant.

1 Like
9

There are two versions I tried and both are working:

What @stevepiercy proposed and the test side by side with the file its testing

.
├── docs
└── src
    └── foo
        └── bar
            ├── mything.py
            ├── mything_test.py
            ├── otherthing.py
            └── otherthing_test.py

Above works fine for small and well organized projects. Advantage: To be tested and test are side by side, no fiddling with two locations.

For Plone as with its many files I would do it like @stevepiercy proposed, which is the other method I use. Above is no longer convenient if there are many files per folder, plus zcml, templates, ....

I agree to no include test in the wheels.

1 Like
10

I don't like leaving the tests out of the distributions released on PyPI. It makes it hard to run the tests for all Plone packages together, the way we have historically always done on Jenkins using buildout.coredev. We are running into this already with the packages that use pytest -- see plone.distribution tests are not found in coredev (all pytest tests?) · Issue #978 · plone/buildout.coredev · GitHub

As far as I am aware, there is no requirement that pytest tests need to be in one structure or the other. But I have seen some weird things happen if the test discovery finds them in a different place then where the test modules are imported from (which can happen when running tests under tox which installs the package into a virtualenv)

1 Like
11

I prefer to have the tests inside the package, so plone/something/tests, and included in the wheel. The reason is as David says: this is the only way to run all tests of all packages combined.

Alternatives:

  • Stop doing that. In Zope each package is tested individually on GitHub Actions, there is no Jenkins server, and this seems to work fine. But in Plone I think there is too much chance that a fix in one package causes errors in another package and that this would only get discovered after a new Plone release, so too late.
  • On Jenkins (and locally when you use buildout.coredev), always checkout all packages. Then the tests are available because they are in the source, and it should be possible to find them all. Could still be tricky. You would somehow need to point pytest to the right directory. I sat with Robert at the Alpine City Sprint last week, and he updates some coredev files in a draft/example PR; we would need something like that.
  • Somehow force an install from a source dist (tarball) instead of a binary wheel. Then we can make sure (with MANIFEST.in) that tests in a top level folder are included in the source dist. But I think it is not very well defined where on the filesystem these data files would then end up.

For some packages it may be fine to do it a bit differently than the rest of Plone. Possibly we could allow more room for core add-ons (anything living above CMFPlone, so no dependency) to make their own choices. Also, a package like plone.autoinclude has a very special test setup (which needs fixing BTW) and we can't possibly run its tests on Jenkins.

So exceptions could be made, but in general I would be in favour of having the tests inside the package.

1 Like
12

BTW, switching from zope.testrunner to pytest as testrunner should be fine. As long as you have zope.pytestlayer, apparently no changes should be needed, if I understand correctly.

But I do sometimes see pytest and/or zope.pytestlayer added to the [test] dependencies, and I would like to avoid that. The test runner itself should not be a test dependency. Or do I see this wrong and is this somehow needed?
Of course if you actually do import pytest in your tests, or have a conftest.py, which is specific for pytest, then it should be added.

BTW, we cannot use pytest 8 yet, as zope.pytestlayer needs fixes. Help wanted.

1 Like
13

Late to the party, but adding my two cents here.

Location

I do prefer to have tests outside the codebase, the main reasons for that:

  • One central location for all tests, and easily accessible
  • Allow you to organize your tests according to the fixtures you need to run. i.e: have a subfolder named services and, in there, add a conftest.py adopting the funcional test layer to all tests.
  • Easy to manage test resources (json files, test images, etc)

pytest/zope.pytestlayer/pytest-plone

As, in my projects I rely on pytest-plone for fixtures, I always add it as a test requirement -- and it depends on zope.pytestlayer and on pytest.

If we plan to just run the existing unittests with pytest, that is not necessary.

2 Likes
14

Late as well, but my very quick 2 cents:

As long as we can run the tests on released packages, I don't mind where they are.

We have a problem currently in Plone core already though: there are a few core packages (plone.volto, plone.exportimport, plone.classicui, and some others), that are using pytest and zope.testrunner can not run them at all.

This means, that any change on P.CMFPlone that breaks them goes unnoticed.

I was planning to work on it during the Alpine City Sprint, but the testing matrix on plone.meta and plenty of conversations (and breaking Jenkins :sweat: ) took that time away.

I will try to get a first working version of a Jenkins job for those packages maybe tomorrow at the tune up day :crossed_fingers:

2 Likes