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

Feature request: @ChildSuite for breaking down Suites in to namespaced subdomains #296

Closed
adammcarter opened this issue Mar 16, 2024 · 7 comments
Labels
enhancement New feature or request wontfix This will not be worked on

Comments

@adammcarter
Copy link

Description

I'm just looking at integrating Swift Testing in to my new project to avoid the weight of XCTest so apologies if this is covered in the documentation but I can't see it.

I'm breaking down a test suite in to subdomains - a common pattern of a Suite relating to a type and subdomains to break down the components of that type to avoid long winded test names with the pattern SUT.functionName.given.when.then() and instead breaking this down in to child suites namespaced for the SUT and the function name.

For example, if my Suite is for a notifications service, I want to break this down to subdomains per function/variable to neatly break up my tests:

struct NotiicationsService {
    var isNotificationsAvailable: Bool { /*logic*/ == true }

    func queueNotifications() { /*queue notifications if needed*/ }
}

Then in my tests I want to break this up per var/func above in to it's own child suite...

@Suite
struct NotificationsServiceTests {
    var sut: NotificationsService
    
    init() {
        self.sut = .production()
    }
    
    @Suite
    struct IsNotificationsAvailable {
        var sut: NotificationsService
        
        init() {
            self.sut = .production()
        }
    }
    
    @Suite
    struct QueueNotifications {
        var sut: NotificationsService
        
        init() {
            self.sut = .production()
        }
    }
}

extension NotificationsServiceTests.IsNotificationsAvailable {
    @Test
    func returnsTrue_whenAuthorisationStatusIsAuthorized()
        #expect()
    }
    
    @Test
    func returnsFalse_whenAuthorisationStatusIsDenied() {
        #expect()
    }
}

extension NotificationsServiceTests.QueueNotifications {
    @Test
    func addsNotificationRequests_whenNewContentIsAvailable() {
        #expect()
    }
    
    @Test
    func addsNotificationRequests_whenUpdatedContentIsAvailable() {
        #expect()
    }
}

This works in my code but the obvious code smell to anyone looking at this is the need to pass down the sut to each "child" suite.

Is there a better way to do this using the existing Swift Testing framework or is this a potential for a new feature - @ChildSuite?

Looking at the current documentation around organising tests:
https://swiftpackageindex.com/apple/swift-testing/main/documentation/testing/organizingtests

... It's implied that child Suites are encouraged:

In addition to containing test functions and any other members that a Swift type might contain, test suite types can also contain additional test suites nested within them. To add a nested test suite type, simply declare an additional type within the scope of the outer test suite type.

But unless I'm missing something (likely) I can't see any more information in these docs around nesting the Suites - especially for my use case of namespacing the different functionality to group tests on a per function basis.

Proposed solution

It would be nice if we could create a macro for @ChildSuite so it inherits the properties and initialiser/deinit by default to clean this up.

This would look like the below:

@Suite
struct NotificationsServiceTests {
    var sut: NotificationsService
    
    init() {
        self.sut = .production()
    }
    
    @ChildSuite
    struct IsNotificationsAvailable {}
    
    @ChildSuite
    struct QueueNotifications {}
}

extension NotificationsServiceTests.IsNotificationsAvailable {
    @Test
    func returnsTrue_whenAuthorisationStatusIsAuthorized()
        #expect()
    }
    
    @Test
    func returnsFalse_whenAuthorisationStatusIsDenied() {
        #expect()
    }
}

extension NotificationsServiceTests.QueueNotifications {
    @Test
    func addsNotificationRequests_whenNewContentIsAvailable() {
        #expect()
    }
    
    @Test
    func addsNotificationRequests_whenUpdatedContentIsAvailable() {
        #expect()
    }
}

Expected behavior

No response

Actual behavior

No response

Steps to reproduce

No response

swift-testing version/commit hash

0.6.0

Swift & OS version (output of swift --version && uname -a)

swift-driver version: 1.90.11.1 Apple Swift version 5.10 (swiftlang-5.10.0.13 clang-1500.3.9.4)
Target: arm64-apple-macosx14.0
Darwin AT-ALK6L5VQ 23.3.0 Darwin Kernel Version 23.3.0: Wed Dec 20 21:30:44 PST 2023; root:xnu-10002.81.5~7/RELEASE_ARM64_T6000 arm64

@adammcarter adammcarter added the enhancement New feature or request label Mar 16, 2024
@adammcarter
Copy link
Author

Additionally we can do some more things with this with the specific knowledge that the suites are children such as printing the output in the console in a more user friendly way with indentation.

Currently the output can be flat as each test suite is on the top level in the console:

◇ Test run started.
↳ Testing Library Version: unknown (Swift 5.10 toolchain)
◇ Suite NotificationsDeepLinkUrl started.
◇ Test returnsAppPreferencesNotificationsUrl_wheniOS() started.
✔ Test returnsAppPreferencesNotificationsUrl_wheniOS() passed after 0.003 seconds.
✔ Suite NotificationsDeepLinkUrl passed after 0.003 seconds.
◇ Suite QueueNotificationsForRunChangesReturningBranchNames started.
◇ Test returnsEmptyArray_whenOldRunsHasARun_butNotificationsAreNotAvailable() started.
✔ Test returnsEmptyArray_whenOldRunsHasARun_butNotificationsAreNotAvailable() passed after 0.008 seconds.
◇ Test returnsEmptyArray_whenOldRunsIsEmpty() started.
✔ Test returnsEmptyArray_whenOldRunsIsEmpty() passed after 0.002 seconds.
◇ Test returnsNewRun_whenOldRunsHasANewRun_andNotificationsAreAvailable_andNotificationPreferenceIsEveryone() started.
✔ Test returnsNewRun_whenOldRunsHasANewRun_andNotificationsAreAvailable_andNotificationPreferenceIsEveryone() passed after 0.012 seconds.
✔ Suite QueueNotificationsForRunChangesReturningBranchNames passed after 0.023 seconds.
✔ Test run with 4 tests passed after 0.027 seconds.

But armed with the knowledge of child suites we can indent the children:

◇ Test run started.
↳ Testing Library Version: unknown (Swift 5.10 toolchain)
◇ Suite NotificationsServiceTests started. // ++
  ↳ ◇ Suite NotificationsDeepLinkUrl started.
    ◇ Test returnsAppPreferencesNotificationsUrl_wheniOS() started.
    ✔ Test returnsAppPreferencesNotificationsUrl_wheniOS() passed after 0.003 seconds.
  ✔ Suite NotificationsDeepLinkUrl passed after 0.003 seconds.
  ↳ ◇ Suite QueueNotificationsForRunChangesReturningBranchNames started.
    ◇ Test returnsEmptyArray_whenOldRunsHasARun_butNotificationsAreNotAvailable() started.
    ✔ Test returnsEmptyArray_whenOldRunsHasARun_butNotificationsAreNotAvailable() passed after 0.008 seconds.
    ◇ Test returnsEmptyArray_whenOldRunsIsEmpty() started.
    ✔ Test returnsEmptyArray_whenOldRunsIsEmpty() passed after 0.002 seconds.
    ◇ Test returnsNewRun_whenOldRunsHasANewRun_andNotificationsAreAvailable_andNotificationPreferenceIsEveryone() started.
    ✔ Test returnsNewRun_whenOldRunsHasANewRun_andNotificationsAreAvailable_andNotificationPreferenceIsEveryone() passed after 0.012 seconds.
  ✔ Suite QueueNotificationsForRunChangesReturningBranchNames passed after 0.023 seconds.
✔ Test run with 4 tests passed after 0.027 seconds.

@stmontgomery
Copy link
Contributor

Thanks for filing this, @adamcarter93! I'm going to try to separate the specifically proposed solution (a new macro with a distinct name) from the underlying problems/limitations you are describing, to break this down some. It sounds like you are requesting:

  1. A mechanism for nested (or "child") suites to access the stored state of their containing suite types.
  2. That the human-readable console output indent nested/child suites to more clearly represent their hierarchy.

I agree 1 would be very useful, and it may or may not strictly require a distinct macro to accomplish something like this. I think this issue can focus solely on exploring that topic.

Re: item 2, we could actually do this with the information we already have today, since internally the testing library forms a tree to track the hierarchical relationships of @Suite types. The reason we don't do this currently mainly is due to parallelization: suites and tests are parallelized by default and this means that representing this using indented text in the console output would require buffering to wait for earlier suites to finish, and potentially delay output from being shown about other suites which have finished. Nevertheless, this idea may still be worth considering even with those tradeoffs (perhaps when parallelization is disabled or with some other opt-in). I would rank this lower priority but I would encourage filing that as a distinct issue.

@adammcarter
Copy link
Author

I agree 1 would be very useful, and it may or may not strictly require a distinct macro to accomplish something like this. I think this issue can focus solely on exploring that topic.

Sounds good to me! When you say this won't need a distinct macro, what are your thoughts on this? I'm quite new to writing of macros so excuse my ignorance here if there's a technical reason for it. Or do you mean this could be a potential for a different mechanism ie making use of traits instead of a new macro?

Re item 2: Good shout with the parallelisation - I hadn't considered that. An opt-in flag could be interesting or we could go down the route of spitting out the test suite above the parallel test for all new tests that run without their suite on the line above in the console although I imagine this could be quite gnarly at first the output would read nicer.
Agreed with it being a separate idea though - especially as it sounds like it could be implemented today without the need for child suites!

@adammcarter
Copy link
Author

FYI @stmontgomery I've just opened #298 for the indenting feature suggestion so we can focus this issue on the child suites

@adammcarter
Copy link
Author

adammcarter commented Mar 16, 2024

Alternative solution

After a walk there's a potential alternative to explicit @ChildSuite nested types like in the example above -- extensions.

A common pattern for me to break up tests is to have a declaration of a test class with the sut, variables, common setup etc in the top of the file, and below in separate extensions of that test class have each subdomain of tests.
Eg an extension for one function with many test inside it for that one function, etc.

This is something we can build on which would solve the issue of categorisation of tests under subdomains while also avoiding the need to create duplicate sut variables and initialisation of each as in the initial idea above - as there would only be one test struct with all extensions using the same variables and setup.

For example, today I would write something like this

struct MyThing {
    func doSomething() { }

    func doAnotherThing() { }
}

struct MyThingTests {
    var sut: MyThing

    init() {
        sut = .init()
    }
}

extension MyThingTests {
    @Test
    func doSomething_givenSomething_whenSomething_thenThing() {
        #expect(true)
    }

    @Test
    func doSomething_givenSomethingElse_whenSomething_thenThing() {
        #expect(true)
    }
}

extension MyThingTests {
    @Test
    func doAnotherThing_givenAnotherThing_whenSomething_thenThing() {
        #expect(true)
    }

    @Test
    func doAnotherThing_givenAnotherThing_whenSomething_thenThing() {
        #expect(true)
    }
}

But this doesn't scale as well as muddies the test names with the prefixes of the thing I'm testing.

What I'd like to (ignoring the initial proposal at the top of this thread) is something like:

struct MyThing {
    func doSomething() { }

    func doAnotherThing() { }
}

struct MyThingTests {
    var sut: MyThing

    init() {
        sut = .init()
    }
}

@SuiteExtension("doSomething()")
extension MyThingTests {
    @Test
    func givenSomething_whenSomething_thenThing() {
        #expect(true)
    }

    @Test
    func givenSomethingElse_whenSomething_thenThing() {
        #expect(true)
    }
}

@SuiteExtension("doAnotherThing()")
extension MyThingTests {
    @Test
    func givenAnotherThing_whenSomething_thenThing() {
        #expect(true)
    }

    @Test
    func givenAnotherThing_whenSomething_thenThing() {
        #expect(true)
    }
}

Where we can remove the prefixes from each test name and keep the cleanliness of the tests - of course this would be conflicting if we have the same function/test name within the same type MyThingTests.

This has the benefit of scaling well and encouraging more granular testing as it's easy for large test files to be messy and difficult to read and maintain.

It also gives us an option of combining with the above initial proposal but with the format above, where each @ SuiteExtension creates a new nested type named with the passed in string (which would of course need cleaning up - eg we don't want the () in the nested type name).

Alternatively we could not use strings (preferable) and tie the @SuiteExtension macro to key paths to:

  1. Ensure the subdomains are actually a part of the thing being tested
  2. Give support for refactoring to the IDE as strings are often missed out during renaming
  3. Read nicer than hardcoded string
  4. Give an explicit context that the extension is a child of the thing being tested
@SuiteExtension(\.doAnotherThing)
extension MyThingTests {
    @Test
    func givenAnotherThing_whenSomething_thenThing() {
        #expect(true)
    }

    @Test
    func givenAnotherThing_whenSomething_thenThing() {
        #expect(true)
    }
}

@adammcarter
Copy link
Author

Example code for proposed

Expanding on the previous comment, the below shows what a macro could look like before/after expansion

Today - manually hand written out, verbose, noisy

struct MyThing {
    let someProperty = ""
    
    func doSomething() {}
}

@Suite
struct MyTestSuite {
    var sut: MyThing
    
    init() {
        sut = .init()
    }
}

// This isn't clear what the extension is until you read the function prefix
extension MyTestSuite {
    // Boiler plate code with prefix repetition
    @Test
    func someProperty_given_when_then() {}
    
    // Boiler plate code with given/when repetition - potential for bad copy/paste each time
    @Test
    func someProperty_givenOtherThing_when_then() {}
    
    @Test
    func someProperty_givenOtherThing_whenSlightlyDiffererentThing_then() {}
    
    @Test
    func someProperty_givenOtherThing_whenSlightlyDiffererentThing2_then() {}
}

extension MyTestSuite {
    @Test
    func doSomething_given_when_then() {}
    
    @Test
    func doSomething_givenOtherThing_when_then() {}
}

Proposed @SuiteExtension macro definition(s)

  • Can only be added to @Suite types
@SuiteExtension(_ describing: String, context: String = "")
@SuiteExtension(_ describing: KeyPath<Root, Value>, context: String = "")

End goal - before macro expansion

struct MyThing {
    let someProperty = ""
    
    func doSomething() {}
}

@Suite
struct MyTestSuite {
    var sut: MyThing
    
    init() {
        sut = .init()
    }
}

@SuiteExtension(\MyThing.someProperty)
extension MyTestSuite {
    @Test
    func given_when_then() {}
}
    
@SuiteExtension(\MyThing.someProperty, context: "givenOtherThing")
extension MyTestSuite {
    @Test
    func when_then() {}
    
    @Test
    func whenSlightlyDifferentThing_then() {}
    
    @Test
    func whenSlightlyDifferentThing2_then() {}
}

@SuiteExtension("doSomething")
extension MyTestSuite {
    @Test
    func doSomething_given_when_then() {}
    
    @Test
    func doSomething_givenOtherThing_when_then() {}
}

After macro expansion

struct MyThing {
    let someProperty = ""
    
    func doSomething() {}
}

@Suite
struct MyTestSuite {
    var sut: MyThing
    
    init() {
        sut = .init()
    }
}

@SuiteExtension(\MyThing.someProperty)
extension MyTestSuite {
    // Generated struct based on the keypath from the `describing` macro parameter, with sut, init and test functions with names expanded to the `describing` macro parameter
    @Suite
    struct MyThing_SomeProperty {
        var sut: MyThing
        
        init() {
            sut = .init()
        }
        
        @Test
        func myThing_someProperty_given_when_then() {}
    }
}

@SuiteExtension(\MyThing.someProperty, context: "givenOtherThing")
extension MyTestSuite {
    // Generated struct based on the keypath from the `describing` macro parameter, with sut, init and test functions with names expanded to the `describing` AND the `context` macro parameter
    @Suite
    struct MyThing_SomeProperty_GivenOtherThing {
        var sut: MyThing
        
        init() {
            sut = .init()
        }
        
        @Test
        func myThing_someProperty_givenOtherThing_someProperty_givenOtherThing_when_then() {}
        
        @Test
        func myThing_someProperty_givenOtherThing_someProperty_givenOtherThing_whenSlightlyDifferentThing_then() {}
        
        @Test
        func myThing_someProperty_givenOtherThing_someProperty_givenOtherThing_whenSlightlyDifferentThing2_then() {}
    }
}

@SuiteExtension("doSomething")
extension MyTestSuite {
    // Generated struct based on string name from the `describing` macro parameter, with sut, init and test functions with names expanded to the `describing` AND the `context` macro parameter
    @Suite
    struct DoSomething {
        var sut: MyThing
        
        init() {
            sut = .init()
        }
        
        @Test
        func doSomething_given_when_then() {}
        
        @Test
        func doSomething_givenOtherThing_when_then() {}
    }
}

@grynspan
Copy link
Contributor

Unfortunately macros cannot accomplish what you want here. They are, by design, unable to see members of containing types.

Even if they could do it, there's a huge amount of nuance involved. What do you do if two parent types both have a name field? What do you do if one parent type is actor-isolated and another is actor-isolated to a different actor? What do you do if a type is embedded in an extension to another type, in which case the original declaration is unavailable? These questions do not have trivial or obvious answers.

It seems like you're sort of reinventing polymorphism here. We don't currently support non-final classes as suites, but that's a technical constraint we hope to eventually lift, and subclassing would probably solve the problem.

Failing that, consider grouping tests into extensions on the same type rather than placing them in different types entirely. Or, place common members in a helper type and include a property of that type on your suite types.

@grynspan grynspan added the wontfix This will not be worked on label Mar 16, 2024
@grynspan grynspan closed this as not planned Won't fix, can't repro, duplicate, stale Mar 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

3 participants