Clarify fixture execution order and provide visual aids

[SalmonMode]

I noticed in the docs, it mentions that fixtures are executed in the order a test requests them (unless scope, dependencies, or autouse are involved), but this can be very misleading depending on how the tests and fixtures are set up, especially as a test suite gets more complex.

The only truly guaranteed way to control order is by leveraging scope, dependencies, and autouse, and making sure these 3 things make up a linearizable map of fixtures for each test.

The changes I made to the docs clarify this by mentioning it explicitly. I also broke down the order section into 3 parts (one for each of those ordering mechanisms), and provided several examples and images to help visualize it.

I also gave a similar treatment to the fixture availability section as it tied into the changes I made to the fixture order section. Plus, it was easy to repurpose some of the SVGs I made already.

The changes have certain implications, because fixture request order was the documented behavior, so some are still depending on it. But, given the internals of pytest, and how scope/dependencies/autouse have been the only ways to guarantee order for a while, the users that are currently dependent on that behavior have already been at risk of that behavior breaking down due to test execution order/scope/dependencies/autouse, and they will be at the same risk (or lack thereof) of it breaking down after this documentation change (especially since I learned my lesson about sets not having a deterministic iteration order 😂 , and we'll pretty much always iterate over fixture requests in the order they were requested by the test).

So ultimately, this doc change doesn't actually change anything for those users, and provides clearer, more helpful information for anyone looking to learn how to structure their fixtures.

Side note: I changed usage of the word "instantiate"/"instantiation" in these sections to "execute"/"execution", because it more closely reflects how pytest currently does things with fixture defs, and "execute" is easier to understand IMO.

[bluetech]

Hi @SalmonMode, since the RTD status seems to work now, can you rebase this PR? Then it will be easier to review for those of use who are too lazy to build locally 😁

[SalmonMode]

Definitely! I wanna see the new feature too haha

[SalmonMode]

I'm on mobile at the moment, but I'll squash these commits into one once I get to my computer

[nicoddemus]

Awesome contribution, thanks a lot @SalmonMode!

I've left a few suggestions/minor fixes, otherwise this is a great addition to the docs. 👍

[SalmonMode]

Thanks for looking at this @nicoddemus! I'll squash it into one commit now

[bluetech]

These are really nice explanations. At least I learned some stuff :)

However, I feel they are a too detailed for this document. I think some readers will get lost in these details which are not super important for the majority of cases, especially first time readers. There are also ordering problems (some are preexisting).

Maybe the examples can be moved toward the end, to an "examples" section, or maybe even a new "advanced fixtures" page?

[bluetech]

I think the example can be simplified somewhat.

1. Is there any particular reason to have both TestOne and TestTwo? Wouldn't just one class be sufficient to demonstrate the point?

2. Perhaps the order fixture can be replaced with prints? I think this will be easier to grasp.

[SalmonMode]

The reason for having two classes is to demonstrate that each test class has its own inner fixture, and aren't pulling it from the other class' scope. I also didn't want to involve anything about overriding fixtures just yet because that wasn't what this part was about, and there's a section for that already. In order for these tests to pass, both classes must have their own inner fixture defined.

Regarding order, I pulled this from another example in the docs. I think it's ideal because it demonstrates an actual implication for the tests, whereas prints don't impact the tests. That said, actually seeing it may be beneficial as well. Maybe I should add a note about running it with --setup-show/plan?

I also used the order style of example everywhere in this change, and I think having a common style where the tests are impacted can help the user to understand more concepts further down the page and help build their mental model of how it works and how they can structure the tests/fixtures.

[bluetech]

The example is great, even I didn't know this would work and I'm supposedly a core dev ;)

However, I feel the placement of this section is too early; it appears very near the top. If I'm trying to get into the mindset of a reader who doesn't know anything about pytest fixtures, so open up the official doc and starts reading it top to bottom, the example demonstrates something they are quite unlikely to ever encounter, but it requires some mental effort to understand.

[SalmonMode]

I'm sure many users will read this from top to bottom once through, or maybe even a few times. But I partially wrote this from the perspective of someone who is coming back to the docs to find out the details of some behavior/system or to get ideas about how to structure their tests/fixtures.

I think the only reason this isn't something the user will likely encounter, is because this behavior isn't brought to light in the docs. Some users may hold the belief that if a fixture isn't available from the perspective of the fixture requesting it, it won't work. But the general idea is somewhat common. A similar concept is in Python itself with the NotImplementedError exception.

But I agree that this may be stepping up things a little too quickly provided what came before it. I think there should be better descriptions of the basics of the fixture system above it. The concept of "requesting" a fixture doesn't seem to be covered, and things are discussed in a more technical sense at the beginning of the page (e.g. Test functions can receive fixture objects by naming them as an input argument). I think going over those sorts of concepts, and terminology first with very simple examples can prime them for the behavior described in this part.

[bluetech]

This section uses the word scope in the meaning of "the set of places in which the fixture is available". In later sections the word scope is used in the meaning "the set of places in which the fixture instantiation will be reused".

This possibly sets up the reader for some confusion?

[SalmonMode]

I see the distinction you're mentioning, but in this case, I think it's helpful for the user's mental model to have it be used both ways. The scopes draw boundaries between where fixtures can be used/re-used as well as where they can be defined; it's two sides of the same coin. The visual aides I put in are meant to help illustrate this.

The only scope they can't also be defined in is the function level, but things get a little janky there IMO anyway (e.g. parameterization affects in both directions, as opposed to just the fixtures that come after a parameterized fixture).

[SalmonMode]

@bluetech I can move the autouse fixture section up and revamp things a bit to cover the basics a bit better. Should that be done in this PR? or should I make a separate one?

[nicoddemus]

However, I feel they are a too detailed for this document. I think some readers will get lost in these details which are not super important for the majority of cases, especially first time readers. There are also ordering problems (some are preexisting).

I agree, I had the same impression when first reading this. I intended to get back to it with a more concrete proposal where to put some of the sections explained here, but didn't find the time.

Maybe the examples can be moved toward the end, to an "examples" section, or maybe even a new "advanced fixtures" page?

I think a new "advanced fixtures" docs would be a nice place to put it.

[SalmonMode]

@nicoddemus @bluetech I restructured it quite a bit and added a bunch of more introductory material to the beginning before diving in to stuff. I tried to go over the basics first, and then ramp up to more advanced concepts. Let me know what you think 😁

[bluetech]

@SalmonMode Wanted to say I didn't forget about this -- I am planning to review this again, but probably only for 6.1, as it needs proper attention.

[SalmonMode]

No worries!

[nicoddemus]

(I took the liberty of rebasing due to #7424)

Whew! Sorry for taking so long to review this, I knew the review needed to be done carefully in order to assess all the hard work you have done here.

Overall great work! You touch in a bunch of highly advanced topics here, which were not really present anywhere else, so they are definitely valuable.

I've left a bunch of small fixes and a few suggestions.

Reading the document from top to bottom is still not great, but mostly because things have been added to it in an organic matter. However I don't believe we need to fix this in this PR: we can sort this top-to-bottom flow in later PRs, as leaving large PRs open like this is often a source of conflicts.

I also believe many of the advanced topics here should be put in a separate section, but again I feel this should be done in later PRs as to avoid stalling this much longer.

Again, thanks a lot for all the hard work put into this!

[nicoddemus]

I particularly don't like that the main "Act" of the test is being done by the fixture, but I understand the purpose here was to show the delete_email call after the yield.

However I think this is not necessary from a logical point of view (receiving_user is deleted already), so I think this is clearer:

    @pytest.fixture
    def email():
        return Email(subject="Hey!", body="How's it going?")

    def test_email_received(sending_user, receiving_user, email):
        sending_user.send_email(email, receiving_user)
        assert email in receiving_user.inbox

Or even:

    def test_email_received(sending_user, receiving_user):
        email = Email(subject="Hey!", body="How's it going?")
        sending_user.send_email(email, receiving_user)
        assert email in receiving_user.inbox

[nicoddemus]

If this is applied, the same logic should be applied to the addfinalizer section below

[SalmonMode]

This was partially to keep it consistent with the approach where a test class is used to represent the end state after the act, and each test method can assert something about that state. In order for that approach to work, the act had to be done in a fixture. But I also didn't want to assume referential integrity in the DB.

Granted, this is documentation for a test framework, not an imaginary email system, so no reason to over-engineer it 😂

I'll go ahead and swap it around.

[SalmonMode]

Hmmm after re-reading it, this wouldn't allow for demonstrating the bit about them tearing down in the reverse order because there's no clear setup order, beyond the order the users were requested by the test, but I'd rather not depend on that.

Maybe we should get rid of the reverse order stuff above and put a more apparent reference to the "safe teardown" section? But IMO, that's one of the core features of yield fixtures. What do you think?

[SalmonMode]

@nicoddemus Thanks for the review! Sorry I took so long to respond. I left some comments for you. Let me know what you think when you get a chance! 😁

[bluetech]

Would omit the quotes here, I don't think they are necessary.

[bluetech]

Behavior is not something that can be empirically measured

I guess you are not a fan of the field of Psychology :) But more seriously, a test is exactly empirically measuring of behavior, so this statement doesn't sound right to me.

[SalmonMode]

I see where you're coming from, and agree.

I'm thinking of something like logging in. Someone may want to test logging in, but there's no means of saying assert user.logged_in is True and have that statement be 100% correct. Instead, they may look at how the system responds to the context of a proper login request bring sent. They may check that they now have a cookie, or that they can access a resource only a logged in user should be able to access. But these still don't tell them if the user is logged in, because being logged in is an abstract concept we've attached to the way we want the code to behave given certain conditions.

Perhaps it would be better to say something like this?

The intent or meaning behind how the state behaves is not something that can be empirically measured, which is why writing tests can be challenging.

[bluetech]

A reader is likely to not know what "SUT" means here.

[bluetech]

This paragraph and the note after the next paragraph are a bit too post-modern IMO. To be a bit blunt: as an official technical document, I think our aim should be to bring clarity and precision, not to subvert readers' notions of popular testing aphorisms, or reinforce the existential angst of young impressionable Python programmers :)

[bluetech]

This sentence is repetitive with the one in the Fixture errors section.

[bluetech]

I don't see an email fixture defined. And sending_user is used without being requested.

[nicoddemus]

Btw: if we see other examples in the docs, we can see # content of XXX.py: that's not only informative, but is actually used by regendoc to run that example. Also blocks that start with $ are considered shell commands, and the output is appended to the text below it.

One can use tox -e regen locally to get that output updated.

[nicoddemus]

Thanks again @SalmonMode to sticking to this!

I believe we should solve any outstanding issues like the ones brought by @bluetech and then merge this. We can then make further improvements (like perhaps moving bits of this doc to an "advanced fixture" section or something) in a separate PR. As it stands this is pretty big already and hard to track further changes, and already an improvement on its own.

[SalmonMode]

I agree. I'll work on this today now that I've got some extra time on my hands.

[nicoddemus]

Thanks @SalmonMode!

I think this is in good enough shape to merge as is, we can improve this later if need be (like moving some pieces to an "Advanced section" for example).

[SalmonMode]

Awesome! I'll squash everything and rebase today. Might be a bit tricky though as it seems I've made quite the mess with the commits haha!

[SalmonMode]

looks like the rebase went iffy. I'll get it fixed up tomorrow.

[SalmonMode]

@nicoddemus all set to be merged! 😁

[nicoddemus]

Thanks @SalmonMode!

@bluetech would you like to take a final look?

[nicoddemus]

Going ahead and merge this, we can do further improvements in a new PR.

Thanks a lot @SalmonMode for the contribution and bearing with us!

[SalmonMode]

My pleasure!

[nicoddemus]

Backport: #8160

[The-Compiler]

@SalmonMode Out of curiosity, how did you make those SVGs? Judging from the comments in there, are those hand-written? 🤯

Would it be okay if I perhaps incorporate some of those (in a modified form) into my pytest trainings? I conduct those trainings at conferences like Europython or PyConDE, but I also sometimes give them for-profit in companies.

Of course I'd be happy to name you in the last slide or something, that's the least I can do!

[SalmonMode]

@The-Compiler yeah, I handwrote them. And thanks!

I don't see a problem with it. I think it might be more appropriate to reference the pytest docs, rather than me, though haha.

[ssbarnea]

@SalmonMode I loved these diagrams too and I was hoping there was a tool that could be used to facilitate building these. Sadly, I do not have the patience to hand write diagrams myself.