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

support swift-testing in MockManager.fail #518

Merged
merged 4 commits into from
Nov 3, 2024

Conversation

arovge
Copy link
Contributor

@arovge arovge commented Oct 23, 2024

Resolves #517

Testing

In FailTest.swift, I copied all the XCTest test cases and migrated them to Swift Testing. These tests will now fail if an error wasn't recorded during their run.

Mixing XCTest/Swift Testing usages in the other frameworks tests

If both Testing and XCTest can be imported, this change will cause every call to MockManager.fail to call both Issue.record() and XCTFail().

Since a test case can only ever be a XCTest or a Swift Testing test, I wanted to verify this wouldn't cause any issues calling one testing failure function when using the other framework.

I created/ran some adhoc tests locally to verify there were no issues with using XCTFail() in a Swift Testing test and and vice versa when using Issue.record() in a XCTest

Both test suites passed successfully. No other unexpected issues occurred so I believe this is a safe change.

Code:

class XCTestWithTestingIssue: XCTestCase {
    func test_withIssue() {
        Issue.record("This test will still pass with no issues")
    }
}

struct TestingWithXCTestFail {
    @Test
    func withXCTFail() {
        XCTFail("This test will still pass with no issues")
    }
}

@MatyasKriz
Copy link
Collaborator

Hey, didn't know Swift Testing was usable alongside XCTests. The changes look good.

I'll take a look on my machine.

@arovge
Copy link
Contributor Author

arovge commented Oct 25, 2024

I just thought to check what testing packages like Nimble do for reporting test errors in swift-testing and XCTest. Here's their PR that added support for it: Quick/Nimble#1154

It looks like you can somewhat rely on Test.current != nil to determine if you're running in a swift-testing test, but that breaks when Task.detached { } is used in the test

There's an open issue to support this in swift-testing: swiftlang/swift-testing#475

Let me know if you're interested in following this more targeted approach until swift-testing has more support for this. The current approach seems like it's a noop to always report XCTest/swift-testing failures, but it'd definitely be nicer to try being detecting what test framework we're running in. Especially if this changes in the future

@MatyasKriz
Copy link
Collaborator

Alright, sorry about the delay, had to force myself away from Factorio. 😄

Runs just the same from what I can see locally.

However, could I ask you to add some sample tests to StubTest.swift for example? So that we have passing SwiftTesting tests as well as failing ones.

@arovge
Copy link
Contributor Author

arovge commented Oct 31, 2024

Can do! I've added swift-testing variants of each test in that file

@MatyasKriz
Copy link
Collaborator

Looks great, I'll merge it and release right away. 🙂 Thank you for your contribution!

@MatyasKriz MatyasKriz merged commit 904ac6b into Brightify:master Nov 3, 2024
@arovge
Copy link
Contributor Author

arovge commented Nov 5, 2024

@MatyasKriz I meant to ask about this when I first opened the PR but it completely slipped my mind until updating my version of Cuckoo today - my apologies not asking when this PR was open

There's one other usage of XCTFail in OCMock: https://github.com/Brightify/Cuckoo/blob/master/OCMock/ObjectiveAssertThrows.swift#L15

I also did a project wide search for XCTAssert, but the only usages are in tests so this should be the only remaining spot that would need to be updated for full swift-testing support

I'd be happy to submit another PR to complete the swift-testing support! But I might need some tips for building OCMock locally. I'm getting No such module 'OCMock' after a successful make and I'm not sure how to proceed

@MatyasKriz
Copy link
Collaborator

@arovge makes sense, I didn't realize that OCMock has its own method for failing tests.

Thanks for notifying me about the OCMock problem, it must have started in the previous version and slipped by me.

Either way, the current master contains a fix for it, so you should be able to run make again and then the OCMock tests will succeed.

@arovge
Copy link
Contributor Author

arovge commented Nov 18, 2024

Thanks that did the trick and got my build to work!

I hadn't realized at the time, but objcVerify is defined as an extension to XCTestCase:

public func objcVerify<OUT>(_ invocation: @autoclosure () -> OUT, _ quantifier: OCMQuantifier = .exactly(1), file: StaticString = #file, line: UInt = #line) {

Since any test it's used in would need to be an XCTest, any OCMock usage in tests shouldn't suffer from the same problem of silently passing when running with swift-testing. Hopefully that's acceptable since I don't have any Objective-C experience to help add swift-testing support 😆

Sorry for the confusion with this last bit & thanks for the quick responses with getting this PR merged. It's greatly appreciated 😃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Swift Testing verify is not working
2 participants