Skip to content

struct's inits are executed before each test, but class's deinits only after a suite is complete. #605

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

Closed
pitt500 opened this issue Aug 7, 2024 · 8 comments
Labels
bug 🪲 Something isn't working concurrency Swift concurrency/sendability issues

Comments

@pitt500
Copy link

pitt500 commented Aug 7, 2024

Description

Hi, I'm learning how to use Swift Testing and preparing a series of tutorials for my Youtube Channel.

In my next video I want to present how to migrate from setUp and tearDown into init/deinit. According to this documentation struct's init acts as XCTest's setUp, and I created a small demo to see what is the time when init is called:

var myValue = 0

struct ProductStoreTest {
    init () {
        print("init called")
        myValue = 0
    }
    
    @Test
    func test1() {
        print("Test1 executed")
        myValue += 10
        #expect(myValue == 10)
    }
    
    @Test
    func test2() {
        print("Test2 executed")
        myValue += 10
        #expect(myValue == 10)
    }
    
    @Test
    func test3() {
        print("Test3 executed")
        myValue += 10
        #expect(myValue == 10)
    }
}
Screenshot 2024-08-07 at 10 58 53 a m

As you can see, init is called before each test runs, which is expected. However, since deinit is not available on structs, I have to convert my suite into a final class. I was expecting that deinit will be executed after each test run, but that's not happening:

Screenshot 2024-08-07 at 12 34 45 p m Screenshot 2024-08-07 at 10 58 17 a m

So, my question is: Is this expected?

From what I'm seeing, deinit is running after all tests in the suite are done. If that's the intention, I feel this is confusing in terms of marking deinit as equivalent to tearDown.

I will really appreciate your guidance. Thank you!

Expected behavior

Class's deinit work as teardown running after each test is complete.

Actual behavior

Class's deinit are executed after all tests in suite are done.

Steps to reproduce

  1. Run the tests above in a new project as final class (not struct).
  2. Observe

swift-testing version/commit hash

0.11.0

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

  • Language version: Swift 5
  • Minimum Deployment Target: iOS 17.0
  • Xcode 16 Beta 5
@pitt500 pitt500 added the bug 🪲 Something isn't working label Aug 7, 2024
@grynspan
Copy link
Contributor

grynspan commented Aug 7, 2024

Tests in Swift Testing run in parallel by default, meaning that they will be accessing the shared global state myValue at the same time. Your code won't compile in Swift 6 mode and in Swift 5 mode I'd expect to see a diagnostic such as:

⚠️ Var 'myValue' is not concurrency-safe because it is nonisolated global shared mutable state

@grynspan
Copy link
Contributor

grynspan commented Aug 7, 2024

Because tests run in parallel by default, the exact moment at which deinit is called relative to another test is undefined (or, at least, controlled by the system's task/thread/queue/etc. scheduler.)

If you run your tests with the --no-parallel option, do you see logging occur in the order you expect?

@pitt500
Copy link
Author

pitt500 commented Aug 7, 2024

Tests in Swift Testing run in parallel by default, meaning that they will be accessing the shared global state myValue at the same time. Your code won't compile in Swift 6 mode and in Swift 5 mode I'd expect to see a diagnostic such as:

⚠️ Var 'myValue' is not concurrency-safe because it is nonisolated global shared mutable state

@grynspan Thanks, that makes sense. Right now I'm just running a quick test to try out Swift Testing. That global variable won't be part of my "production" code.

@grynspan grynspan added the concurrency Swift concurrency/sendability issues label Aug 7, 2024
@grynspan
Copy link
Contributor

grynspan commented Aug 7, 2024

Verified at desk that the logging occurs at the expected time when --no-parallel is passed. This is just a side effect of running tests in parallel: deinit is called at the correct/expected time, but because multiple tests are running at once, it can appear that events occur in spooky order.

I'm closing this issue on that basis, but please do feel free to reopen if I've misunderstood the problem or if I've missed something here!

@grynspan grynspan closed this as not planned Won't fix, can't repro, duplicate, stale Aug 7, 2024
@pitt500
Copy link
Author

pitt500 commented Aug 7, 2024

@grynspan I tried running the suite serialized, but I got this issue that I don't know how to fix:
Screenshot 2024-08-07 at 4 43 21 p m

I tried cleaning everything and nothing worked. The only way to fix it is creating a brand new project from Xcode and selecting Swift Testing as Testing Framework, but if it's added as a dependency, this issue happens all the time. (Is this already reported?)

Regarding your comment, that makes sense, however, this documentation is misleading about the use of deinit, since it's not acting as tearDown. In that case, I would recommend to update the documentation to clarify that deinit != tearDown.

Thanks for your help!

@grynspan
Copy link
Contributor

grynspan commented Aug 8, 2024

@grynspan I tried running the suite serialized, but I got this issue that I don't know how to fix:

I tried cleaning everything and nothing worked. The only way to fix it is creating a brand new project from Xcode and selecting Swift Testing as Testing Framework, but if it's added as a dependency, this issue happens all the time. (Is this already reported?)

We haven't yet tagged a release of the Swift Testing package that includes the .serialized trait (see #535), so the compiler is complaining that it doesn't look like a display name instead (because it has no idea .serialized refers to a trait.) The fork of Swift Testing included in Xcode already exposes the trait as API. I was planning to tag 0.12.0 on Monday and it will include this trait.

Regarding your comment, that makes sense, however, this documentation is misleading about the use of deinit, since it's not acting as tearDown. In that case, I would recommend to update the documentation to clarify that deinit != tearDown.

Respectfully, the difference in behaviour here is entirely due to the parallelization of tests when using Swift Testing. If we implemented tearDown() in Swift Testing exactly as it's implemented in XCTest, you'd still see the same or similar behaviour because tests would still be setting themselves up in parallel, running in parallel, and tearing themselves down in parallel. I hope that makes sense.

@pitt500
Copy link
Author

pitt500 commented Aug 8, 2024

We haven't yet tagged a release of the Swift Testing package that includes the .serialized trait (see #535), so the compiler is complaining that it doesn't look like a display name instead (because it has no idea .serialized refers to a trait.) The fork of Swift Testing included in Xcode already exposes the trait as API. I was planning to tag 0.12.0 on Monday and it will include this trait.

I didn't know that. Thanks for sharing!

Respectfully, the difference in behaviour here is entirely due to the parallelization of tests when using Swift Testing. If we implemented tearDown() in Swift Testing exactly as it's implemented in XCTest, you'd still see the same or similar behaviour because tests would still be setting themselves up in parallel, running in parallel, and tearing themselves down in parallel. I hope that makes sense.

And thank you for the explanation. I completely understand this behavior, and I apologize if my previous comment was not clear enough. My point is that I believe it would be beneficial if the documentation included a note indicating that due to the nature of parallelization, it's not simply a matter of replacing tearDown with deinit, but rather refactoring your code to function in a parallel environment or making it serialized in the meantime.

At the end, it's merely a suggestion that could assist other developers. Thank you for taking the time to reply to my issue! 😃

@grynspan
Copy link
Contributor

grynspan commented Aug 8, 2024

We may be talking past each other, so I'm sorry if I'm just being boneheaded here and not understanding.

This isn't specific to deinit and tearDown: init will also run concurrently with Swift Testing when setUp wouldn't, as will test bodies, as will any other knobs and levers on your tests and suites. Tautologically, tests running in parallel will tend to run all parts of their code in parallel.

Developers using Swift 6 are generally going to be guided by the compiler to write concurrency-safe code, and so long as your code is concurrency-safe, the fact that deinit might run in the middle of another test's execution should have no real impact on the tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🪲 Something isn't working concurrency Swift concurrency/sendability issues
Projects
None yet
Development

No branches or pull requests

2 participants