Reconsider afterEach execution order #989
Comments
|
Hey @ikesyo 👋🏼 😄 In the interest of time and getting v5 out sooner, I'm going to bump this to v6. |
|
FWIW, I maintain a branch with a working implementation of this idea here: pcantrell/Quick@around-each...pcantrell:logical-aftereach-order See especially this diff in the specs, which does a good job of explaining the effects of the change. That branch could become a PR if we decide we want to implement this. |
jessesquires
pushed a commit
that referenced
this issue
Apr 14, 2022
This is a reopening of #820, brought up to date with the latest main. Here is an updated summary; full historical discussion is in the original PR. I’d sure love to get this merged before it hits the 4-year mark! --- This PR adds an `aroundEach` decoration, which accepts the individual example as a callback: ```swift aroundEach { example in ... example() ... } ``` See the similar features in [Rspec](https://relishapp.com/rspec/rspec-core/v/3-0/docs/hooks/around-hooks) and [JUnit](https://junit.org/junit4/javadoc/4.12/org/junit/rules/RuleChain.html#around(org.junit.rules.TestRule)). ## Motivation There are a few test setup/cleanup scenarios the existing `beforeEach`/`afterEach` don’t handle, or don’t handle well. Some test decorations can’t split into a separate `beforeEach` and `afterEach`. The original motivation for adding `aroundEach` is [Siesta checking for leaks in specs](https://github.com/bustoutsolutions/siesta/blob/2b7bff709e0938a593de1f08415e905b4eece3ab/Tests/Functional/ResourceSpecBase.swift#L115-L145). To do this, it needs to wrap each spec in its own autorelease pool, then verify that objects are no longer retained _after_ the pool is drained: ```swift it("does this") { autoreleasepool { doThis() } checkObjectsNoLongerRetained() } it("does that") { autoreleasepool { doThat() } checkObjectsNoLongerRetained() } ``` I’d like to pull out both the `autoreleasepool` block _and_ `checkObjectsNoLongerRetained()` so they’re not repeated in each spec. (The two are tightly coupled, and it makes sense to factor them out together.) This is not possible with `beforeEach`/`afterEach`, but with `aroundEach` it is: ```swift aroundEach { example in autoreleasepool { example() } checkObjectsNoLongerRetained() } it("does this") { doThis() } it("does that") { doThat() } ``` Other methods that take a closure/block present a similar problem: XCTest’s `measure`, for example. Even in cases where it is possible to split the decoration into `beforeEach` and `afterEach`, it’s not always desirable. Grouping tightly coupled before and after behavior in a single `aroundEach` can help ensure proper nesting of operations. ## Implementation notes This PR works to preserve Quick’s somewhat illogical before/after hook execution order. (See #989.) This PR reimplements _before_ and _after_ hooks using the new feature. Internally, there is now only _around_. This appears to work; all existing specs still pass! ## Merge checklist - [x] Tests - [x] Documentation - [x] Objective-C support - [x] Ensure new implementation of before/after does not break existing public behavior - [x] Appease SwiftLint - [x] Is this a new feature (Requires minor version bump)?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
ref: #820 (comment)
On the other hand, XCTestCase's
addTeardownBlocksays:I'm considering that making the behavior to match with XCTest would be desirable/easy to understand.
The text was updated successfully, but these errors were encountered: