Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace nameerror_after_exception by pytests's continue_on_failure #88

Open
Sheila-nk opened this issue Aug 14, 2023 · 4 comments
Open

Comments

@Sheila-nk
Copy link
Collaborator

Sheila-nk commented Aug 14, 2023

#87 (comment) please make sure that this test passes with the plugin

Originally posted by @ev-br in #87 (comment)

@ev-br
Copy link
Member

ev-br commented Feb 24, 2024

This came first in scipy/scipy#13116 , might help cooking up a standalone test case.
Once there is a test, it'll be easier to decide if this can be merged with --continue_on_failure in the plugin layer.

@ev-br
Copy link
Member

ev-br commented Feb 26, 2024

A test exists for the doctest layer in test_testmod.py::TestNameErrorAfterException. The CLI incantation is

$ pytest scpdt/tests/failure_cases_2.py --doctest-modules --doctest-continue-on-failure

and it indeed seems to match: with --doctest-continue-on-failure the traceback has four exceptions (as expected for name_error_after_exception=True); without it we only get one.

So need to golf sync the DTConfig setting and the pytest setting:

  • if the pytest switch is on, it overwrites the DTConfig one (maybe emit a warning on mismatch)
  • if the pytest switch is off, it is synced from DTConfig.

@ev-br ev-br assigned ev-br and unassigned ev-br Feb 26, 2024
@ev-br
Copy link
Member

ev-br commented Feb 26, 2024

Looked at it a while. This is low prio TBH.

A worthу aim would be to remove PytestDTRunner.report_unexpected_exception and .report_failure which were vendored from pytest's runner, https://github.com/pytest-dev/pytest/blob/8.0.2/src/_pytest/doctest.py#L184

However, this would require to multiple inherit PytestDTRunner from both DebugDTRunner (which it does now) and pytest's PytestDoctestRunner.

Does not seem worth it TBH.

Also, the PytestDTRunner inherits from DebugDTRunner, so it stops after the first exception anyway. Also, DebugDTRunner overrides all logic from nameerror_after_exeption.

@ev-br
Copy link
Member

ev-br commented Aug 24, 2024

From the code health perspective, it's probably best to simply remove the nameerror_after_exception and always use the pytest naming, continue_on_failure throughout, both in pytest and "basic" doctest layer.

@ev-br ev-br changed the title Add test case for NameError after exception Replace nameerror_after_exception by pytests's continue_on_failure Aug 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants