Rule request: [Test class having setup method, but no teardown] #3452
Comments
|
It's a common case for not having a class StorageTests: XCTestCase {
var storage: StorageProtocol!
func setUp() {
storage = InMemoryStorage()
}
func testSomething() {
...
}
}In this case, there wouldn't be any side effects since there's a brand new external dependency generated before each test. And the old one will be successfully deallocated by ARC, so memory consumption is also not an issue. Such rule will make more harm forcing developers to have redundant empty |
|
I found the original article pointing to me that teardown are necessary: https://qualitycoding.org/xctestcase-teardown/ protocol StorageProtocol {
var i: Int { get }
}
class InMemoryStorage: NSObject, StorageProtocol {
var i = 1
deinit {
print("\(self) gets deinit")
}
}
class StorageWithTearDownTests: XCTestCase {
var storage: StorageProtocol!
deinit {
print("\(self) gets deinit")
}
override func setUp() {
storage = InMemoryStorage()
}
override func tearDown() {
storage = nil
}
func testSomething() {
XCTAssertTrue(storage.i == 1)
}
}
class StorageTests: XCTestCase {
var storage: StorageProtocol!
deinit {
print("\(self) gets deinit")
}
override func setUp() {
storage = InMemoryStorage()
}
func testSomething() {
XCTAssertTrue(storage.i == 1)
}
}
/// Z prefix to be the last test runned
class ZdebugMemoryGraph: XCTestCase {
func testZLastTestToRun_debugMemoryGraph_allowingToFindMemoryLeaks() {
usleep(1000)
XCTAssertTrue(true)
}
}Only one deinit gets called: |
|
Whoa, I didn't know that. Additionally checked the sources in |
|
That's one reason why I this such a rule should be added and opted-in |
|
Hi @kenji21, let me know if the PR makes sense to you :-) |
|
Looks nice, you have covered all cases ( #3495 (comment) ) |
New Issue Checklist
New rule request
Please describe the rule idea, format
this issue's title as
Rule Request: [Rule Name]and describe:It's a good pratique to "rollback" in teardown what we do in setup on unit test
And XCtest create an instance of the test class for each method, if we don't set objects to nil in the teardown method, we use more memory than necessary, and attribute not setted back to nil can make side effects
Provide several examples of what would and wouldn't trigger violations.
setUp(orsetUpWithError) withouttearDown(ortearDownWithError()) method in a file with Test/Tests in the end of its name (convention)Should the rule be configurable, if so what parameters should be configurable?
No i don't see any possible configuration
Should the rule be opt-in or enabled by default? Why?
To lower memory consumption in tests, and prevent side effects
The text was updated successfully, but these errors were encountered: