ScreenPyHQ / screenpy

Screenplay pattern base for Python automated UI test suites.
MIT License
28 stars 4 forks source link

The sample test cases provided in the docs are not isolated #35

Closed ramoscav-nokia closed 1 year ago

ramoscav-nokia commented 1 year ago

test_comedic_timing fails unless it runs after test_dramatic_moment.

$ pytest -k test_comedic_timing

# Sample output:
>       assert_that(value, self.resolution, reason)
E       AssertionError: 
E       Expected: 'laughter'
E            but: was 'tense'

Ideally, connect_to_audience() should be mocked on every test case; something similar to:

@patch('pollster.connect_to_audience')
def test_comedic_timing(pollsterMock, Cameron: AnActor, Polly: AnActor) -> None:
    pollsterMock.poll_mood.top_mood = 'laughter'
    ...
perrygoy commented 1 year ago

Whoa, nice catch, and thanks for the issue! I'll be able to address this in the small docs update i'm going to do, unless someone else gets to it first. :D

perrygoy commented 1 year ago

I think it should be fixed now, see this change.

I don't think we mention the conftest.py file in the docs, so i think this is the only place that needed to be updated. Let me know if we missed a spot @ramoscav-nokia, and thanks again for submitting this issue!

Edit: Woops, well... we do mention the conftest.py file in the docs, here. But we don't mention the mocked cam_py or pollster libraries, so i think the only way to actually run the full example suite is to run it from screenpy_examples.

perrygoy commented 1 year ago

Seems like this is probably fixed. If you disagree, please reopen this!

ramoscav-nokia commented 1 year ago

Thanks for taking the time addressing this issue, @perrygoy.

I'd like to suggest a slightly different approach where mocks are defined within the tests so that it is closer to a real world scenario.

This following example is closer to the ideal solution I requested when first opening the issue and highlights how one could configure the mock per test case; this way the pytest hook in conftest.py is no longer needed.

# features/test_mood.py
...
from pollster import laughter_packet, tense_packet, connect_to_audience
from functools import wraps

def mock_packet(packet):
    def decorator_mock_packet(func):
        @wraps(func)
        def wrapper_func(*args, **kwargs):
            connect_to_audience().poll_mood.side_effect = [packet]
            func(*args, **kwargs)
        return wrapper_func
    return decorator_mock_packet

@mock_packet(tense_packet)
def test_dramatic_moment(Cameron: AnActor, Polly: AnActor) -> None:
    """We can use the camera to create dramatic tension."""
    Cameron.has_ordered_cleanup_tasks(StopRecording())
    ...

@mock_packet(laughter_packet)
def test_comedic_timing(Cameron: AnActor, Polly: AnActor) -> None:
    """We can use the camera to make funny moments."""
    Cameron.has_independent_cleanup_tasks(StopRecording())
    ...

When I first opened this issue I was unfamiliar with how mocking works in python, but thanks to your fix I was able to figure out how to decorate these tests and provide the mocks directly.

perrygoy commented 1 year ago

That looks pretty slick, i like it!

However, would you be open to writing your own pull request to address those changes? I'd love for your name to be attached to your suggested fix. Plus it's super cool.

If you're not comfortable with that, i can do it for you instead!

rcavaz commented 1 year ago

Of course @perrygoy, it'll be a pleasure. I'll be using this account for the PR.

perrygoy commented 1 year ago

@rcavaz actually, on second thought, the goal for the sample test there is to show how the test would look using a custom library we invented. Those mocks would not be necessary in a "real world" scenario, where these libraries (pollster, cam_py) were real. The weird backstage magic in the pollster library is actually OK in my book to make the actual test file look like it's supposed to.

I do really like the decorator-style mocks, though, so i forgot the goal of that example suite. Does the backstage magic not meet your needs?

rcavaz commented 1 year ago

There are real world scenarios where I may want to write screenpy tests with mocks.

For those with limited understanding of mocks in python or who are new to the Screenplay pattern will greatly benefit from such examples and should be able to adopt them faster.

perrygoy commented 1 year ago

Hm... i think i'm OK with it as long as there's a nice docstring on the decorator explaining that it's needed to make our fake library work. :D

perrygoy commented 1 year ago

To summarize the discussion this ticket started, it would be nice to have examples that showed some form of mocking, but the mocking suggested by @rcavaz didn't sit right with me because it felt like mocking out the whole "application" under test. I think we need a separate example whose purpose is to demonstrate using mocking with ScreenPy.

I'm not sure what the best example would be—maybe a test against a website which has some (fake or real) external service to grab details from, and we mock that service to get the test user?

perrygoy commented 1 year ago

Thanks again for the discussion and perspective, @ramoscav-nokia! I'm going to close this issue in favor of the work in https://github.com/ScreenPyHQ/screenpy_examples/issues/1.