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

Improve test expressivity #101

Merged
merged 5 commits into from
Aug 28, 2021
Merged

Improve test expressivity #101

merged 5 commits into from
Aug 28, 2021

Conversation

BasThomas
Copy link
Contributor

Using specific assert functions will generate easier to understand failure messages if the tests were to fail. For example, in the XCTAssertNil case, it will say "something was unexpectedly not nil" instead of only "test failed".

Using specific assert functions will generate easier to understand failure messages if the tests were to fail. For example, in the `XCTAssertNil` case, it will say "something was unexpectedly not nil" instead of only "test failed".
@@ -290,7 +290,7 @@ final class MockerTests: XCTestCase {
let simpleURL = URL(string: "https://host.com/api/resource")
let mock = Mock(url: composedURL, dataType: .json, statusCode: 200, data: [.get: MockedData.exampleJSON.data])
let urlRequest = URLRequest(url: simpleURL!)
XCTAssertEqual(composedURL.absoluteString, simpleURL!.absoluteString)
XCTAssertEqual(composedURL.absoluteString, simpleURL?.absoluteString)
XCTAssert(mock == urlRequest)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one I didn't change; somehow it is comparing a Mock to a URLRequest... but XCTAssertEqual didn't seem to be able to figure out the generics, so I left it as-is.

Copy link
Contributor

@AvdLee AvdLee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a very welcome change! I couldn't agree more, using the right assertions for the job will help a lot 🙂

guard let data = data, let image: UIImage = UIImage(data: data) else {
XCTFail("Invalid data")
XCTFail("Invalid data \(String(describing: data))")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might want to fix this @BasThomas

Copy link
Contributor

@kairadiagne kairadiagne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work, a small change that makes working with the tests much better!

The SwiftLint configuration would otherwise complain with the following error:

> 🚫 variable declared in 'guard' condition is not usable in its body
@wetransferplatform
Copy link
Collaborator

wetransferplatform commented Aug 26, 2021

Warnings
⚠️

Consider to place some MARK: lines for Sources/Mock.swift, which is over 200 lines big.

Messages
📖

View more details on Bitrise

📖 Mocker: Executed 25 tests, with 0 failures (0 unexpected) in 1.864 (1.922) seconds

MockerTests.xctest: Coverage: 95.75

File Coverage
MockerTests.swift 95.73%

Mocker.framework: Coverage: 88.04

File Coverage
MockingURLProtocol.swift 86.41%
Mock.swift 82.18%

Generated by 🚫 Danger Swift against 985b218

AvdLee
AvdLee previously requested changes Aug 26, 2021
Copy link
Contributor

@AvdLee AvdLee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BasThomas can you have a look at the trailing whitespace warnings?

@BasThomas
Copy link
Contributor Author

😢 yes. I sweat I have the setting enabled in Xcode. Wonder if something is broken.

@BasThomas
Copy link
Contributor Author

Also weird as I do not seem to have changed them? 🤔

@BasThomas
Copy link
Contributor Author

swiftlint autocorrect it is.

@BasThomas
Copy link
Contributor Author

I don't understand... Bitrise is unhappy, but I can't find the attached run that would have failed :(

@BasThomas BasThomas merged commit 37f27d3 into WeTransfer:master Aug 28, 2021
@BasThomas BasThomas deleted the improve-test-expressivity branch August 28, 2021 09:07
@wetransferplatform
Copy link
Collaborator

Congratulations! 🎉 This was released as part of Release 2.5.4 🚀

Generated by GitBuddy

1 similar comment
@wetransferplatform
Copy link
Collaborator

Congratulations! 🎉 This was released as part of Release 2.5.4 🚀

Generated by GitBuddy

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.

4 participants