scipy / scipy_doctest

Floating-point aware doctesting
BSD 3-Clause "New" or "Revised" License
5 stars 4 forks source link

Run tests in context managers #87

Closed Sheila-nk closed 1 year ago

Sheila-nk commented 1 year ago

Closes #82

Sheila-nk commented 1 year ago

Bug It seems the DebugDTRunner implementation is not raising the defined exceptions as it should. This is likely because of this part of the code in the DoctestRunner run function:

       if out is None:
            encoding = save_stdout.encoding
            if encoding is None or encoding.lower() == 'utf-8':
                out = save_stdout.write
            else:
                # Use backslashreplace error handling on write
                def out(s):
                    s = str(s.encode(encoding, 'backslashreplace'), encoding)
                    save_stdout.write(s)

From the code implementation here in the DTRunner

    def _report_item_name(self, out, item_name, new_line=False):
        if item_name is not None:
            out("\n " + item_name + "\n " + "-"*len(item_name))
            if new_line:
                out("\n")

out is used as a callable but the raised TypeError indicates that it is still a list object

ev-br commented 1 year ago

What does DebugDTRunner have to do with any of this? Its only job is to stop after the first failure, so it' pytest -x.

Sheila-nk commented 1 year ago

I opted to override its run function since it is a subclass of DTRunner and calls the run function of its base class. Also I noticed pytest's doctest module uses the DebugRunner as well here.

Sheila-nk commented 1 year ago

I can override the DTRunner instead. I am still a bit unsure at which points either runner is called.

ev-br commented 1 year ago

Ah OK, this might make sense if pytest runs doctests one by one needs to override the reporting. Let's come back to this after the weekend.

EDIT: For usual doctests, DoctestRunner is the main thing which gets a list of doctests, runs them and reports the progress and results. DebugDoctestRunner stops after the first failure. DTRunner and DebugDTRunner follow this.

Sheila-nk commented 1 year ago

It would probably be a good idea to make continue_on_failure configurable through ini options.

Sheila-nk commented 1 year ago

@ev-br does nameerror_after_exception config attrbute serve the same purpose as the continue_on_failure variable? I can utilize that instead.

Edit: it actually does. So I will use nameerror_after_exception instead. Users can set it in conftest.py.

ev-br commented 1 year ago

@ev-br does nameerror_after_exception config attrbute serve the same purpose as the continue_on_failure variable? I can utilize that instead.

Not sure actually. I don't know what continue_on_failure is; The job of nameerror_after_exception originally comes from the scipy issue linked in the commit message of https://github.com/ev-br/scpdt/pull/35/commits/88d8ed710ab2cc35dfc0c8a99274fff7c6abbd1f .

EDIT: the relevant test is https://github.com/ev-br/scpdt/pull/35/commits/88d8ed710ab2cc35dfc0c8a99274fff7c6abbd1f#diff-e8cfb69580bba75d7bb6f9acb8dc3bd180fb424fdf130732869e621004217492R101

Sheila-nk commented 1 year ago

@ev-br I have made all the recommended changes except for:

  1. the declaration of the PytestDTRunner class and import of doctest at the module level. When I moved everything to the module level, it seems all references to doctest is from the _pytest module which brings up this error:
    AttributeError: module '_pytest.doctest' has no attribute 'UnexpectedException'
  2. I have moved the creation of a test-case for the Name Error after exception to a new issue. I am yet to figure out what the test case looks like but manual testing by setting the namerror_after_exception config attribute in conftest.py is working as expected.
ev-br commented 1 year ago

LGTM, thank you Sheila!

Let's not worry about cleanups at this stage, I'll do a holistic review later anyway.