Lemmons / pytest-raises

An implementation of pytest.raises as a pytest.mark fixture
MIT License
19 stars 8 forks source link

docker update, setup_raises, match argument, "coverage" #13

Closed svenevs closed 5 years ago

svenevs commented 6 years ago

How it All Works

This is also detailed thoroughly in the docstrings, but here's the overview of how it all works together.

  1. Share as much code as possible between raises and setup_raises markers.
    • Hook impl executed (either pytest_runtest_setup for setup_raises or pytest_runtest_call for raises like usual).
    • item, outcome, and string marker_name indicating whether its coming from setup or call forwarded to _pytest_raises_validation.
  2. If marker setup_raises or raises are found, there are now three distinct scenarios tested.
    • Case 1: test raised exception, and it isinstance of the expected exception class (either via exception=ExceptionClass or the default catch-all Exception if unspecified.
      • If the message or match are specified, the exception message is then validated.
      • If the message or match do not check out, _pytest_fail_by_mark_or_set_excinfo is called.
    • Case 2: test raised an exception, but it was of an unexpected type.
      • _pytest_fail_by_mark_or_set_excinfo fails the test.
    • Case 3: the test did not raise, but since it was marked with either setup_raises or raises test is failed using _pytest_fail_by_mark_or_set_excinfo.
  3. Distinctions must be made between setup_raises and raises. I don't understand the details (hence the pytest issue linked above), but basically there are problems with setting outcome.excinfo or calling pytest.fail(...).
    • The distinctions between the two are made because we do not want tests to ERROR, we want them to FAIL. See pytest/#4175 for details discovered by trial-and-error.
  4. So to deal with this, I create a "handshake" using an intentionally obtusely named marker for the setup_raises so that tests properly FAIL rather than ERROR. The general idea is: during pytest_runtest_setup, cases 1-3 are "flagged" with a secret marker, which is then checked for during pytest_runtest_call. Search secret_marker in code.
  5. Since (4) had to go down like this, I leveraged the same trick to also add some checks on things like is the exception=??? actually valid, etc.

I wouldn't go as far as to say the implementation is particularly clean, but it works reliably and I really need it. This is the revisited implementation of what I was discussing in #9.

There is a small concern. Consider the newly added test_pytest_mark_setup_raises_unexpected_exception_fixture in test_raises.py:

import pytest

class SomeException(Exception):
    pass

class AnotherException(Exception):
    pass

@pytest.fixture
def raise_me():
    raise AnotherException('the message')

@pytest.mark.setup_raises(exception=SomeException)
def test_pytest_mark_setup_raises_unexpected_exception_fixture(raise_me):
    pass

In any case where setup_raises is used, the test function body should be empty (just a single pass to compile). Logically, I cannot think of any scenario where if you expect setup_raises you also want to have an implementation in the test function. But the implications are:

  1. If you have testing code in the function body and are banking on your setup raising, this is a contradiction.
  2. You therefore cannot mark a function with both @pytest.mark.raises and @pytest.mark.setup_raises. Only one is allowed -- you either are expecting the setup to fail, in which case the test body should be empty, or you want the test body to fail, in which case the test setup better not raise since you need a valid test setup.

Anyway, this is a big change and should definitely be reviewed. I hesitated to put them all in one PR, but at the end of the day the docker / pylint fixes were needed for me to test things, and adding the match independently in the context of the changes for setup_raises would make for a guaranteed merge conflict between the two.

I look forward to updating / fixing / changing anything you need. If you don't want this change in this repo then I'll package my own because I can't fully test my code without setup_raises :scream: