Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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 #4200

Closed
mildm8nnered opened this issue Sep 11, 2022 · 6 comments
Labels
enhancement Ideas for improvements of existing features and rules. good first issue Issue to be taken up by new contributors yet unfamiliar with the project.

Comments

@mildm8nnered
Copy link
Collaborator

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
Copy link
Collaborator Author

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 :-(

@SimplyDanny
Copy link
Collaborator

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 SimplyDanny added enhancement Ideas for improvements of existing features and rules. good first issue Issue to be taken up by new contributors yet unfamiliar with the project. labels Sep 11, 2022
@mildm8nnered
Copy link
Collaborator Author

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.

@SimplyDanny
Copy link
Collaborator

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.

@mildm8nnered
Copy link
Collaborator Author

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
Copy link
Collaborator Author

#4262 resolves this, so closing ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Ideas for improvements of existing features and rules. good first issue Issue to be taken up by new contributors yet unfamiliar with the project.
Projects
None yet
Development

No branches or pull requests

2 participants