Add opt-in rule balanced_xctest_lifecycle #3495

otaviocc · 2021-01-17T17:39:02Z

This PR implements balanced_xctest_lifecycle, requested in #3452. This rule enforces test classes to have balanced set-up and tear-down methods.

Triggering examples:

final class FooTests: XCTestCase {
    override func setUp() { /* ... */ }
}

final class FooTests: XCTestCase {
    override func setUpWithError() throws { /* ... */ }
}

Non-triggering examples:

final class FooTests: XCTestCase {
    override func setUp() { /* ... */ }
    override func tearDown() { /* ... */ }
}

final class FooTests: XCTestCase {
    override func setUpWithError() throws { /* ... */ }
    override func tearDown() { /* ... */ }
}

final class FooTests: XCTestCase {
    override func setUp() { /* ... */ }
    override func tearDownWithError() throws { /* ... */ }
}

final class FooTests: XCTestCase {
    override func setUpWithError() throws { /* ... */ }
    override func tearDownWithError() throws { /* ... */ }
}

SwiftLintBot · 2021-01-17T18:02:24Z

	50 Warnings
⚠️	This PR introduced a violation in Alamofire: /Tests/BaseTestCase.swift:29:7: warning: Balanced XCTest life-cycle Violation: Test classes must implement balanced setUp and tearDown methods. (balanced_xctest_lifecycle)
⚠️	This PR introduced a violation in Alamofire: /Tests/ProtectedTests.swift:62:13: warning: Balanced XCTest life-cycle Violation: Test classes must implement balanced setUp and tearDown methods. (balanced_xctest_lifecycle)
⚠️	This PR introduced a violation in Firefox: /ClientTests/FileAccessorTests.swift:9:7: warning: Balanced XCTest life-cycle Violation: Test classes must implement balanced setUp and tearDown methods. (balanced_xctest_lifecycle)
⚠️	This PR introduced a violation in Firefox: /ClientTests/ProfileTest.swift:18:7: warning: Balanced XCTest life-cycle Violation: Test classes must implement balanced setUp and tearDown methods. (balanced_xctest_lifecycle)
⚠️	This PR introduced a violation in Firefox: /ClientTests/BreachAlertsTests.swift:48:7: warning: Balanced XCTest life-cycle Violation: Test classes must implement balanced setUp and tearDown methods. (balanced_xctest_lifecycle)
⚠️	This PR introduced a violation in Firefox: /ClientTests/LoginsListSelectionHelperTests.swift:8:7: warning: Balanced XCTest life-cycle Violation: Test classes must implement balanced setUp and tearDown methods. (balanced_xctest_lifecycle)
⚠️	This PR introduced a violation in Firefox: /ClientTests/LoginsListDataSourceHelperTests.swift:10:7: warning: Balanced XCTest life-cycle Violation: Test classes must implement balanced setUp and tearDown methods. (balanced_xctest_lifecycle)
⚠️	This PR introduced a violation in Firefox: /ClientTests/UIImageViewExtensionsTests.swift:14:7: warning: Balanced XCTest life-cycle Violation: Test classes must implement balanced setUp and tearDown methods. (balanced_xctest_lifecycle)
⚠️	This PR introduced a violation in Firefox: /ClientTests/LoginsListViewModelTests.swift:11:7: warning: Balanced XCTest life-cycle Violation: Test classes must implement balanced setUp and tearDown methods. (balanced_xctest_lifecycle)
⚠️	This PR introduced a violation in Firefox: /ClientTests/DownloadQueueTests.swift:8:7: warning: Balanced XCTest life-cycle Violation: Test classes must implement balanced setUp and tearDown methods. (balanced_xctest_lifecycle)
⚠️	This PR introduced a violation in Firefox: /ClientTests/TabToolbarHelperTests.swift:9:7: warning: Balanced XCTest life-cycle Violation: Test classes must implement balanced setUp and tearDown methods. (balanced_xctest_lifecycle)
⚠️	This PR introduced a violation in Firefox: /StorageTests/RustLoginsTests.swift:10:7: warning: Balanced XCTest life-cycle Violation: Test classes must implement balanced setUp and tearDown methods. (balanced_xctest_lifecycle)
⚠️	This PR introduced a violation in Firefox: /StorageTests/TestBrowserDB.swift:15:7: warning: Balanced XCTest life-cycle Violation: Test classes must implement balanced setUp and tearDown methods. (balanced_xctest_lifecycle)
⚠️	This PR introduced a violation in Firefox: /StorageTests/TestSQLiteRemoteClientsAndTabs.swift:136:7: warning: Balanced XCTest life-cycle Violation: Test classes must implement balanced setUp and tearDown methods. (balanced_xctest_lifecycle)
⚠️	This PR introduced a violation in Firefox: /StorageTests/DiskImageStoreTests.swift:12:7: warning: Balanced XCTest life-cycle Violation: Test classes must implement balanced setUp and tearDown methods. (balanced_xctest_lifecycle)
⚠️	This PR introduced a violation in Firefox: /L10nSnapshotTests/L10nBaseSnapshotTests.swift:11:7: warning: Balanced XCTest life-cycle Violation: Test classes must implement balanced setUp and tearDown methods. (balanced_xctest_lifecycle)
⚠️	This PR introduced a violation in Firefox: /XCUITests/ScreenGraphTest.swift:8:7: warning: Balanced XCTest life-cycle Violation: Test classes must implement balanced setUp and tearDown methods. (balanced_xctest_lifecycle)
⚠️	This PR introduced a violation in Firefox: /SharedTests/RollingFileLoggerTests.swift:10:7: warning: Balanced XCTest life-cycle Violation: Test classes must implement balanced setUp and tearDown methods. (balanced_xctest_lifecycle)
⚠️	This PR introduced a violation in Firefox: /SyncTests/MockSyncServerTests.swift:13:7: warning: Balanced XCTest life-cycle Violation: Test classes must implement balanced setUp and tearDown methods. (balanced_xctest_lifecycle)
⚠️	This PR introduced a violation in Firefox: /SyncTests/MetaGlobalTests.swift:44:7: warning: Balanced XCTest life-cycle Violation: Test classes must implement balanced setUp and tearDown methods. (balanced_xctest_lifecycle)
⚠️	This PR introduced a violation in Kickstarter: /bin/StringsScript/Tests/StringsScriptTests/StringsScriptTests.swift:6:13: warning: Balanced XCTest life-cycle Violation: Test classes must implement balanced setUp and tearDown methods. (balanced_xctest_lifecycle)
⚠️	This PR introduced a violation in Kickstarter: /Library/ApplePayCapabilitiesTests.swift:8:13: warning: Balanced XCTest life-cycle Violation: Test classes must implement balanced setUp and tearDown methods. (balanced_xctest_lifecycle)
⚠️	This PR introduced a violation in Kickstarter: /Library/DataSource/ValueCellDataSourceTests.swift:29:22: warning: Balanced XCTest life-cycle Violation: Test classes must implement balanced setUp and tearDown methods. (balanced_xctest_lifecycle)
⚠️	This PR introduced a violation in Kickstarter: /Library/ViewModels/DiscoveryProjectCategoryViewModelTests.swift:7:7: warning: Balanced XCTest life-cycle Violation: Test classes must implement balanced setUp and tearDown methods. (balanced_xctest_lifecycle)
⚠️	This PR introduced a violation in WordPress: /WordPress/WordPressTest/PrepublishingNudgesViewControllerTests.swift:6:7: warning: Balanced XCTest life-cycle Violation: Test classes must implement balanced setUp and tearDown methods. (balanced_xctest_lifecycle)
⚠️	This PR introduced a violation in WordPress: /WordPress/WordPressTest/RemoteFeatureFlagTests.swift:4:7: warning: Balanced XCTest life-cycle Violation: Test classes must implement balanced setUp and tearDown methods. (balanced_xctest_lifecycle)
⚠️	This PR introduced a violation in WordPress: /WordPress/WordPressTest/PostActionSheetTests.swift:8:7: warning: Balanced XCTest life-cycle Violation: Test classes must implement balanced setUp and tearDown methods. (balanced_xctest_lifecycle)
⚠️	This PR introduced a violation in WordPress: /WordPress/WordPressTest/RegisterDomainDetailsSectionTests.swift:4:7: warning: Balanced XCTest life-cycle Violation: Test classes must implement balanced setUp and tearDown methods. (balanced_xctest_lifecycle)
⚠️	This PR introduced a violation in WordPress: /WordPress/WordPressTest/PushAuthenticationServiceTests.swift:5:7: warning: Balanced XCTest life-cycle Violation: Test classes must implement balanced setUp and tearDown methods. (balanced_xctest_lifecycle)
⚠️	This PR introduced a violation in WordPress: /WordPress/WordPressTest/NotificationReplyStoreTests.swift:8:7: warning: Balanced XCTest life-cycle Violation: Test classes must implement balanced setUp and tearDown methods. (balanced_xctest_lifecycle)
⚠️	This PR introduced a violation in WordPress: /WordPress/WordPressTest/FeatureFlagTests.swift:116:7: warning: Balanced XCTest life-cycle Violation: Test classes must implement balanced setUp and tearDown methods. (balanced_xctest_lifecycle)
⚠️	This PR introduced a violation in WordPress: /WordPress/WordPressTest/PostCompactCellTests.swift:6:7: warning: Balanced XCTest life-cycle Violation: Test classes must implement balanced setUp and tearDown methods. (balanced_xctest_lifecycle)
⚠️	This PR introduced a violation in WordPress: /WordPress/WordPressTest/SiteAddressServiceTests.swift:5:7: warning: Balanced XCTest life-cycle Violation: Test classes must implement balanced setUp and tearDown methods. (balanced_xctest_lifecycle)
⚠️	This PR introduced a violation in WordPress: /WordPress/WordPressTest/Extensions/NotificationCenterObserveOnceTests.swift:6:7: warning: Balanced XCTest life-cycle Violation: Test classes must implement balanced setUp and tearDown methods. (balanced_xctest_lifecycle)
⚠️	This PR introduced a violation in WordPress: /WordPress/WordPressTest/CookieJarTests.swift:5:7: warning: Balanced XCTest life-cycle Violation: Test classes must implement balanced setUp and tearDown methods. (balanced_xctest_lifecycle)
⚠️	This PR introduced a violation in WordPress: /WordPress/WordPressTest/MediaServiceTests.swift:4:7: warning: Balanced XCTest life-cycle Violation: Test classes must implement balanced setUp and tearDown methods. (balanced_xctest_lifecycle)
⚠️	This PR introduced a violation in WordPress: /WordPress/WordPressTest/BasePostTests.swift:6:7: warning: Balanced XCTest life-cycle Violation: Test classes must implement balanced setUp and tearDown methods. (balanced_xctest_lifecycle)
⚠️	This PR introduced a violation in WordPress: /WordPress/WordPressTest/ReaderCardTests.swift:6:7: warning: Balanced XCTest life-cycle Violation: Test classes must implement balanced setUp and tearDown methods. (balanced_xctest_lifecycle)
⚠️	This PR introduced a violation in WordPress: /WordPress/WordPressTest/MediaPicker/Tenor/TenorAPIResponseTests.swift:6:7: warning: Balanced XCTest life-cycle Violation: Test classes must implement balanced setUp and tearDown methods. (balanced_xctest_lifecycle)
⚠️	This PR introduced a violation in WordPress: /WordPress/WordPressTest/DomainCredit/DomainCreditEligibilityTests.swift:4:7: warning: Balanced XCTest life-cycle Violation: Test classes must implement balanced setUp and tearDown methods. (balanced_xctest_lifecycle)
⚠️	This PR introduced a violation in WordPress: /WordPress/WordPressTest/QueueTests.swift:4:7: warning: Balanced XCTest life-cycle Violation: Test classes must implement balanced setUp and tearDown methods. (balanced_xctest_lifecycle)
⚠️	This PR introduced a violation in WordPress: /WordPress/WordPressTest/SiteCreation/SiteAssemblyViewTests.swift:5:7: warning: Balanced XCTest life-cycle Violation: Test classes must implement balanced setUp and tearDown methods. (balanced_xctest_lifecycle)
⚠️	This PR introduced a violation in WordPress: /WordPress/WordPressTest/SiteCreation/SiteAssemblyServiceTests.swift:4:7: warning: Balanced XCTest life-cycle Violation: Test classes must implement balanced setUp and tearDown methods. (balanced_xctest_lifecycle)
⚠️	This PR introduced a violation in WordPress: /WordPress/WordPressTest/SiteCreation/SiteCreatorTests.swift:8:7: warning: Balanced XCTest life-cycle Violation: Test classes must implement balanced setUp and tearDown methods. (balanced_xctest_lifecycle)
⚠️	This PR introduced a violation in WordPress: /WordPress/WordPressTest/PushAuthenticationManagerTests.swift:6:7: warning: Balanced XCTest life-cycle Violation: Test classes must implement balanced setUp and tearDown methods. (balanced_xctest_lifecycle)
⚠️	This PR introduced a violation in WordPress: /WordPress/WordPressTest/PostCompactCellGhostableTests.swift:6:7: warning: Balanced XCTest life-cycle Violation: Test classes must implement balanced setUp and tearDown methods. (balanced_xctest_lifecycle)
⚠️	This PR introduced a violation in WordPress: /WordPress/WordPressTest/RegisterDomainDetailsViewModelLoadingStateTests.swift:4:7: warning: Balanced XCTest life-cycle Violation: Test classes must implement balanced setUp and tearDown methods. (balanced_xctest_lifecycle)
⚠️	This PR introduced a violation in WordPress: /WordPress/WordPressTest/CoreDataHelperTests.swift:10:7: warning: Balanced XCTest life-cycle Violation: Test classes must implement balanced setUp and tearDown methods. (balanced_xctest_lifecycle)
⚠️	This PR introduced a violation in WordPress: /WordPress/WordPressTest/RegisterDomainDetailsViewModelTests.swift:21:7: warning: Balanced XCTest life-cycle Violation: Test classes must implement balanced setUp and tearDown methods. (balanced_xctest_lifecycle)
⚠️	This PR introduced a violation in WordPress: /WordPress/WordPressTest/Collection+RotateTests.swift:4:7: warning: Balanced XCTest life-cycle Violation: Test classes must implement balanced setUp and tearDown methods. (balanced_xctest_lifecycle)

	12 Messages
📖	Linting Aerial with this PR took 1.99s vs 2.0s on master (0% faster)
📖	Linting Alamofire with this PR took 2.89s vs 2.83s on master (2% slower)
📖	Linting Firefox with this PR took 9.78s vs 9.87s on master (0% faster)
📖	Linting Kickstarter with this PR took 15.41s vs 15.41s on master (0% slower)
📖	Linting Moya with this PR took 1.37s vs 1.38s on master (0% faster)
📖	Linting Nimble with this PR took 1.27s vs 1.27s on master (0% slower)
📖	Linting Quick with this PR took 0.57s vs 0.54s on master (5% slower)
📖	Linting Realm with this PR took 4.2s vs 4.21s on master (0% faster)
📖	Linting SourceKitten with this PR took 1.04s vs 1.04s on master (0% slower)
📖	Linting Sourcery with this PR took 8.54s vs 8.52s on master (0% slower)
📖	Linting Swift with this PR took 11.41s vs 11.49s on master (0% faster)
📖	Linting WordPress with this PR took 18.84s vs 18.74s on master (0% slower)

Generated by 🚫 Danger

otaviocc · 2021-01-29T17:54:18Z

Rebased :-)

marcelofabri · 2021-02-05T00:15:19Z

Source/SwiftLintFramework/Rules/Lint/RequiredXCTestTearDownRule.swift

+        name: "Required XCTest Tear Down",
+        description: "Test classes must implement tearDown when setUp is provided.",
+        kind: .lint,
+        nonTriggeringExamples: [


can you add some examples using the class func setUp and class func tearDown?

I wonder if it makes sense to check that they're balanced (e.g. a class setUp requires a class tearDown)

Added :-) It does make sense to check if they're balanced. Since they are static methods they have potential to leak to other tests if misused.

marcelofabri · 2021-02-05T00:18:16Z

FYI addTeardownBlock is another way to solve this problem, but I don't know if it's worth detecting it

otaviocc · 2021-02-05T07:03:42Z

FYI addTeardownBlock is another way to solve this problem, but I don't know if it's worth detecting it

I'm on the fence about this one. Not sure it's worth it.

otaviocc · 2021-02-05T07:04:33Z

@marcelofabri danger failed ˆˆ

marcelofabri · 2021-02-05T18:43:00Z

@marcelofabri danger failed ˆˆ

Can you try pushing an empty commit, something in the changelog, etc, to trigger a new build?

otaviocc · 2021-02-05T23:48:19Z

@marcelofabri danger failed ˆˆ

Can you try pushing an empty commit, something in the changelog, etc, to trigger a new build?

@marcelofabri, done ;-)

otaviocc · 2021-02-08T23:18:03Z

Rebased to resolve a conflict in CHANGELOG.md :-)

otaviocc · 2021-02-10T22:41:01Z

Hi @marcelofabri, can you take a look on this one again? Thanks!

otaviocc · 2021-02-15T19:46:55Z

Hi @marcelofabri, any chance to get this one in the next release? Thanks!

otaviocc · 2021-02-18T08:06:06Z

Hi @jpsim, any chance to get this one merged before the release? Thanks!

jpsim

Nice rule! I do think we should make it a bit more general to handle the inverse case too.

I've just granted you write access to the repo @otaviocc so please merge when you've made those changes (unless they don't make sense for some reason) and CI is green.

Source/SwiftLintFramework/Rules/Lint/RequiredXCTestTearDownRule.swift

otaviocc · 2021-02-25T18:53:13Z

Nice rule! I do think we should make it a bit more general to handle the inverse case too.

I've just granted you write access to the repo @otaviocc so please merge when you've made those changes (unless they don't make sense for some reason) and CI is green.

Thank you for the access! I'll make the changes and will merge then.

to trigger a new build.

otaviocc changed the title ~~Add required_xctest_tearddown opt-in rule~~ Add required_xctest_teardown opt-in rule Jan 17, 2021

kenji21 mentioned this pull request Jan 20, 2021

Rule request: [Test class having setup method, but no teardown] #3452

Closed

2 tasks

otaviocc changed the title ~~Add required_xctest_teardown opt-in rule~~ Add opt-in rule required_xctest_teardown Jan 25, 2021

otaviocc mentioned this pull request Feb 2, 2021

Allow multi-line parameters rule to always break lines #3494

Merged

marcelofabri reviewed Feb 5, 2021

View reviewed changes

jpsim approved these changes Feb 24, 2021

View reviewed changes

Source/SwiftLintFramework/Rules/Lint/RequiredXCTestTearDownRule.swift Outdated Show resolved Hide resolved

otaviocc added 7 commits February 28, 2021 23:48

Add required_xctest_tearddown opt-in rule

bd4dcd5

Add examples for static setUp and tearDown

cdd877c

Add MARKs

3fbc482

to trigger a new build.

Update CHANGELOG after git rebase

a4820f4

Update CHANGELOG after git rebase

7c1b5ff

Add support to unbalanced teardown methods

a6a5e63

Rename Rule to BalancedXCTestLifecycleRule

24122e1

otaviocc changed the title ~~Add opt-in rule required_xctest_teardown~~ Add opt-in rule balanced_xctest_lifecycle Mar 1, 2021

otaviocc merged commit fd97e22 into realm:master Mar 1, 2021

otaviocc deleted the oc/xctests_setup_teardown branch March 1, 2021 01:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add opt-in rule balanced_xctest_lifecycle #3495

Add opt-in rule balanced_xctest_lifecycle #3495

otaviocc commented Jan 17, 2021 •

edited

Loading

SwiftLintBot commented Jan 17, 2021 •

edited

Loading

otaviocc commented Jan 29, 2021

marcelofabri Feb 5, 2021

otaviocc Feb 5, 2021

marcelofabri commented Feb 5, 2021

otaviocc commented Feb 5, 2021

otaviocc commented Feb 5, 2021

marcelofabri commented Feb 5, 2021

otaviocc commented Feb 5, 2021

otaviocc commented Feb 8, 2021

otaviocc commented Feb 10, 2021

otaviocc commented Feb 15, 2021

otaviocc commented Feb 18, 2021

jpsim left a comment

otaviocc commented Feb 25, 2021

Add opt-in rule balanced_xctest_lifecycle #3495

Add opt-in rule balanced_xctest_lifecycle #3495

Conversation

otaviocc commented Jan 17, 2021 • edited Loading

SwiftLintBot commented Jan 17, 2021 • edited Loading

otaviocc commented Jan 29, 2021

marcelofabri Feb 5, 2021

Choose a reason for hiding this comment

otaviocc Feb 5, 2021

Choose a reason for hiding this comment

marcelofabri commented Feb 5, 2021

otaviocc commented Feb 5, 2021

otaviocc commented Feb 5, 2021

marcelofabri commented Feb 5, 2021

otaviocc commented Feb 5, 2021

otaviocc commented Feb 8, 2021

otaviocc commented Feb 10, 2021

otaviocc commented Feb 15, 2021

otaviocc commented Feb 18, 2021

jpsim left a comment

Choose a reason for hiding this comment

otaviocc commented Feb 25, 2021

otaviocc commented Jan 17, 2021 •

edited

Loading

SwiftLintBot commented Jan 17, 2021 •

edited

Loading