SwiftLint icon indicating copy to clipboard operation
SwiftLint copied to clipboard

balanced_xctest_lifecycle, single_test_class, empty_xctest_method and test_case_accessibility do not work on when the parent class is a subclass of XCTestCase

Open mildm8nnered opened this issue 3 years ago • 5 comments

New Issue Checklist

Describe the bug

balanced_xctest_lifecycle, single_test_class, empty_xctest_method and test_case_accessibility work fine for me on XCTestCase subclasses.

However if the parent class is a subclass of XCTestCase, then these rules do not fire.

Complete output when running SwiftLint, including the stack trace and command used
$ Pods/SwiftLint/swiftlint
Linting Swift files in current working directory
Linting 'AppDelegate.swift' (1/4)
Linting 'SwiftLintDemoTests.swift' (2/4)
Linting 'MyTestCase.swift' (3/4)
Linting 'BrokenSwiftLintDemoTests.swift' (4/4)
/Users/martin.redington/Documents/Hudl/Source/SwiftLintDemo/SwiftLintDemoTests/SwiftLintDemoTests.swift:11:7: warning: Balanced XCTest life-cycle Violation: Test classes must implement balanced setUp and tearDown methods. (balanced_xctest_lifecycle)
/Users/martin.redington/Documents/Hudl/Source/SwiftLintDemo/SwiftLintDemoTests/SwiftLintDemoTests.swift:16:5: warning: Empty XCTest Method Violation: Empty XCTest method should be avoided. (empty_xctest_method)
/Users/martin.redington/Documents/Hudl/Source/SwiftLintDemo/SwiftLintDemoTests/SwiftLintDemoTests.swift:11:1: warning: Single Test Class Violation: 2 test classes found in this file. (single_test_class)
/Users/martin.redington/Documents/Hudl/Source/SwiftLintDemo/SwiftLintDemoTests/SwiftLintDemoTests.swift:25:1: warning: Single Test Class Violation: 2 test classes found in this file. (single_test_class)
/Users/martin.redington/Documents/Hudl/Source/SwiftLintDemo/SwiftLintDemoTests/SwiftLintDemoTests.swift:20:5: warning: Test case accessibility Violation: Test cases should only contain private non-test members. (test_case_accessibility)
Done linting! Found 5 violations, 0 serious in 4 files.

Here SwiftLintDemoTests.swift and BrokenSwiftLintDemoTests.swift are identical, except that the classes in BrokenSwiftLintDemoTests.swift inherit from MyTestCase.swift (a subclass of XCTestCase).

Environment

  • SwiftLint version - 0.49.1
  • Installation method used - CocoaPods
  • Paste your configuration file:
# only run on these files
included:
  - SwiftLintDemo
  - SwiftLintDemoTests  

excluded:
  - Pods/

opt_in_rules:
  - balanced_xctest_lifecycle
  - single_test_class
  - empty_xctest_method
  - test_case_accessibility
  • Are you using nested configurations? No
  • Which Xcode version are you using ? 13.4.1 (13F100)
  • Do you have a sample that shows the issue? yes - see attached project
Screen Shot 2022-09-11 at 12 16 46 pm Screen Shot 2022-09-11 at 12 16 57 pm

SwiftLintDemo.zip

mildm8nnered avatar Sep 11 '22 11:09 mildm8nnered

Looking at https://github.com/realm/SwiftLint/blob/main/Source/SwiftLintFramework/Rules/Lint/BalancedXCTestLifecycleRule.swift

it looks like the code is checking that the class being linted does inherit from XCTestCase, even indirectly, here

    private func testClasses(in file: SwiftLintFile) -> [SourceKittenDictionary] {
        file.structureDictionary.substructure.filter { dictionary in
            guard dictionary.declarationKind == .class else { return false }
            return dictionary.inheritedTypes.contains("XCTestCase")
        }
    }

I did try to hack in some debugging, but was unable to build SwiftLint locally :-(

mildm8nnered avatar Sep 11 '22 11:09 mildm8nnered

These rules do not know anything about the type hierarchy. Being AST-based, they only check whether there is XCTestCase in the inheritance list.

It would be an option to add a configuration to all these rules that allows to specify other classes you consider to be test cases.

SimplyDanny avatar Sep 11 '22 11:09 SimplyDanny

So I just had a go at adding an xctestcase_subclasses option to test_case_accessibility, which seemed pretty easy as it already has a configuration, and I got it working on the command line, but I can't get the warnings to appear properly in Xcode for some reason

I have this on a local branch, but was unable to push this branch up, presumably because of github permissions.

mildm8nnered avatar Sep 11 '22 17:09 mildm8nnered

So I just had a go at adding an xctestcase_subclasses option to test_case_accessibility, which seemed pretty easy as it already has a configuration, and I got it working on the command line, but I can't get the warnings to appear properly in Xcode for some reason

Do you call SwiftLint in its own build phase like explained here? That should actually work fine even with a manually compiled SwiftLint binary.

I have this on a local branch, but was unable to push this branch up, presumably because of github permissions.

This repository does not allow direct pushes. Please use a fork and create a pull request when your change is ready to be reviewed.

SimplyDanny avatar Sep 11 '22 20:09 SimplyDanny

Do you call SwiftLint in its own build phase like explained here? That should actually work fine even with a manually compiled SwiftLint binary.

yep, although mine looks like this:

if [ "${CONFIGURATION}" = "Debug" ]; then
    "${PODS_ROOT}/SwiftLint/swiftlint"
fi

This repository does not allow direct pushes. Please use a fork and create a pull request when your change is ready to be reviewed.

will do

mildm8nnered avatar Sep 12 '22 10:09 mildm8nnered

#4262 resolves this, so closing ...

mildm8nnered avatar Dec 06 '22 19:12 mildm8nnered