ScreenPyHQ / screenpy

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

`SeeAnyOf` and `SeeAllOf` have a few inconsistencies. #67

Closed perrygoy closed 1 year ago

perrygoy commented 1 year ago

With the recent merge of #64, @bandophahita noticed that there are a couple ways that SeeAllOf and SeeAnyOf are inconsistent:

I think that the desired behavior is:

bandophahita commented 1 year ago

When executing tests, all tests should be executed (no short-circuiting).

Consider this code:

actor.shall(See(q1, r1))
actor.shall(See(q2, r2))

If the first test fails, pytest will never execute the 2nd test unless they were to wrap it in a try/except. Shouldn't SeeAllOf mimic that same behavior?

perrygoy commented 1 year ago

If the first test fails, pytest will never execute the 2nd test unless they were to wrap it in a try/except. Shouldn't SeeAllOf mimic that same behavior?

Maybe that's one reason to use SeeAllOf instead of a series of Sees, so they can perform each test every time to see the result?

perrygoy commented 1 year ago

After more consideration, i think (personally) that SeeAnyOf() and SeeAllOf() should actually raise an exception for not having any tests. I can only imagine someone put them in like that by mistake, and either a test would be passing when it is missing its assertion or there's just an unnecessary line in their log.

Either way, it doesn't make sense to me that someone would want that intentionally—i think raising an exception to call out the mistake is the best UX.

perrygoy commented 1 year ago

After more consideration, and more conversation with @bandophahita, we've decided to change course on this:

perrygoy commented 1 year ago

Merged and fixed!