Reenable pytest again as part of sage -t

[tobiasdiez]

Pytest used to run on as part of sage -t, but this is no longer the case. Here, we reactivate it by removing some safe-guards that are no longer necessary as pytest is a standard package now.

Moreover, a "runtime error" due to a changing dict in an iteration is fixed in ae60cda.

📝 Checklist

• The title is concise and informative.
• The description explains in detail what this PR is about.
• I have linked a relevant issue or discussion.
• I have created tests covering the changes.
• I have updated the documentation and checked the documentation preview.

⌛ Dependencies

[tobiasdiez]

No idea about the failing tests in test-long....

[orlitzky]

I guess the CI is invoking pytest via the ./sage script and a doctest wrapper? I was just typing pytest like a crazy person.

[tobiasdiez]

I guess the CI is invoking pytest via the ./sage script and a doctest wrapper? I was just typing pytest like a crazy person.

Yes, pytest is called as part of sage -t (that was supposed to work already). I don't get the test failures in dev_tools.py - they seem to be unrelated.

[user202729]

Maybe you should retrigger the CI (e.g. by pushing an empty commit) just in case.

[tobiasdiez]

Maybe you should retrigger the CI (e.g. by pushing an empty commit) just in case.

15f17bb is almost such an empty commit but didn't fix it. I've now opened #40474 for a minimally invasive solution.

[github-actions]

Documentation preview for this PR (built with commit ab4902b; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

[tobiasdiez]

This is now finally working. @user202729 @orlitzky please have a look.

(CI now actually runs the pytests twice: once as we call pytest directly, and once as part of sage -t. This shouldn't be a big issue as the tests complete in very short time. And it makes sure that the pytests are still working even when run from the sage env, so this should give additional security in the setup.)

[user202729]

Hm… I'm not entirely sure what this does, it would be useful to explain what exactly does pytest run on. (methods start with test_ in the file? doctests? methods start with test_ in the whole repository?)

Though it's probably better to answer this in a docstring or rst file edit instead of a comment.

[tobiasdiez]

It's the same as running pytest directly. So, essentially _test.py files and then method with test_ therein.

This PR also doesn't introduce anything new. The functionality to run pytest as part of sage -t is there for some time, it just got broken/disabled by moving conftest.py to the root directory.

[user202729]

I'm not entirely sure that's the right thing to do. Previously sage -t a.py runs doctest in a.py, now it runs doctest in a.py plus *_test.py file in the whole sage directory? At least if it runs a_test.py only I'd call it reasonable.

Plus does it work only from sage root, or any directory?

[tobiasdiez]

I'm not entirely sure that's the right thing to do. Previously sage -t a.py runs doctest in a.py, now it runs doctest in a.py plus *_test.py file in the whole sage directory? At least if it runs a_test.py only I'd call it reasonable.

Yes only _test files are passed on to pytest if you specify a file. To cite from the code:

        # Do not run pytest on individual Python files unless
        # they match the pytest file pattern.  However, pass names
        # of directories.

[orlitzky]

The dev_tools.py error must have something to do with pytest, I also had to fix it in #40971 to get the CI to pass.

[orlitzky]

This is cute but we should probably use os.path.isdir() or not os.path.exists().

[orlitzky]

Aside from that one small refactoring for clarity, this LGTM, but I'm not sure if we should keep the separate pytest CI job. I've been moving slow doctests to pytest, and as they accumulate, it's going to become less and less reasonable to re-run several minutes' worth of tests.