💡 A general pattern we try to follow, is to be very defensive about cleanups to avoid leaking resources. This is particularly important when running tests against AWS, where they might continue incurring cost when leaked.

In that spirit we typically avoid cleanup code like this:

# creating resource_1
# creating dependent resource_2
...
delete_resource_2(...)
delete_resource_1(...)
...

even when wrapped in try/catch blocks.

Instead we have a pytest fixture called cleanups which is a simple list of callables that are called in reverse order of insertion when the test has finished executing.

Given my example above, this would correspond to:

# creating resource_1
...
cleanups.append(lambda: delete_resource_1(...))
 
...

# creating dependent resource_2
...
cleanups.append(lambda: delete_resource_2(...))

This makes sense.

Having looked through my test again, and also looking through some of the existing tests, I think I don't even need to do this via cleanups.

Best as I can tell, looking at the events_create_event_bus fixture here for example, events_create_event_bus actually yields a function which when I call it to create event buses, it also adds them to a list to keep track of them. And on tear down, it cycles through that list, cleaning them up.

So I think I can safely remove the whole cleanup code and as long as I use the existing fixtures to create resources, my tests don't need to explicitly worry with cleaningup.

question: Is there a particular reason this deviates from the cleanup structure in your other tests?

No reason...

To be completely honest, looks like I "forgot" to do it for this test.

In hindsight, after reading your other comment above about cleanups, I now realise that this test is actually the correct one in that as long as resources are created using the existing fixtures, those resources are already tracked and automatically cleaned up at the end of the test.

nit: While asserts are ok to leave in, we typically try to avoid it for testing API responses as it's more brittle than the snapshots and while technically redundant, it does come with an additional maintenance overhead, particularly when rewriting / refactoring a test. For parity tests a good guideline is to keep them mostly for asserting behavior or if you want to be extremely explicit in what edge case the test case is covering.

It's a good point... I just had a habit of asserting at the end of tests, but you're right, since we have the snapshots which do a 1:1 validation, the asserts add no value and I see what you mean about the maintenance cost going forwards.

Will remove these.

This doesn't actually do anything here since we're not capturing any response with a queue ARN

Similar with the other test cases

Yeah I just realised I left a few of these throughout the tests, but you're right, the response only ever contains the rule names, so there's no point in transforming anything other than the rules.

Will remove all the useless ones.

Like with the SQS ARN and EventBus name, we don't capture target ids in the response

Same case as with the SQS ARN, we're not actually capturing a bus name anywhere in the collected snapshots here.

Q: Typically we'd solve this with a transformer. Any particular reason to avoid doing that here, as well as in the other cases?

No reason... I was stuck thinking of the regex transformer which wouldn't work here since the NextToken is different each time.

But since you mentioned this, I now spotted we have a jsonpath transformer which makes this much simpler, since I can now just have a single

        snapshot.add_transformer(
            snapshot.transform.jsonpath(
                jsonpath="$..NextToken",
                value_replacement="<next-token>",
                reference_replacement=True,
            )
        )

And this replaces all instances of NextToken and keeps them numbered when verifying multiple times.

I regenerated the snapshot and it's all good now.