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

XCTAssertNotNil cannot handle optionals #188

Closed
flashspys opened this issue Jul 8, 2017 · 9 comments
Closed

XCTAssertNotNil cannot handle optionals #188

flashspys opened this issue Jul 8, 2017 · 9 comments

Comments

@flashspys
Copy link
Contributor

In your tests you are checking at various locations optionals not to be nil by passing them to the XCTAssertNotNil function. Unfortunally the function is unable to process Swift optionals and will threat even nil-optionals as not-nil. For this reason #187 is crashing and not failing, because first the optional is checked and then force-unwrapped. Another option to check optionals to be nil should be considered, for example as mentioned in this blogpost.

@flashspys
Copy link
Contributor Author

Another option would be to add SwifterSwift's very first XCTest extension 😆

XCTest.assertOptionalNotNil

@SD10
Copy link
Member

SD10 commented Jul 8, 2017

@flashspys Thanks for pointing this out. I think we could also set XCTest to stop testing after failing once. The downside to that is we don't know of any failed tests after this until the next run, but I think it could be a fair compromise.

@flashspys
Copy link
Contributor Author

This will not solve the situation in that a single test is crashing ( != failing) because some force-unwrap fails.

@SD10
Copy link
Member

SD10 commented Jul 8, 2017

@flashspys let me make sure I understand correctly, are you saying this test won't fail:

let nilValue: Int? = nil
XCTAssertNotNil(nilValue)

@flashspys
Copy link
Contributor Author

flashspys commented Jul 8, 2017

I thought, but no. The behaviour is more complicated. I think the following pictures will help to understand:

In this case, XCTAssertNotNil will fail as expected.
screen shot 2017-07-08 at 23 38 57

But in this case XCTAssertNotNil will not detect the nilness of string. The test will crash at the force-unwrap
screen shot 2017-07-08 at 23 39 51

Whyever...

@SD10
Copy link
Member

SD10 commented Jul 8, 2017

@flashspys Ok, I think we're on the same page. That is why I was suggesting this:

  override func setUp() {
    continueAfterFailure = false
  }

If XCTAssertNotNil fails the proceeding code won't be executed.

😅 Actually, I see what you mean now.
So XCTAssertNotNil will not fail in the case where it has more checks after it?

@flashspys
Copy link
Contributor Author

I think it is more an UI Bug than I thought because your idea is right. It will fix it and even in the second example it will stop executing after XCTAssertNotNil when you add continueAfterFailure = false
screen shot 2017-07-09 at 00 00 29
My guess is that Xcode is just not displaying these small red x in case of a runtime error.

And now I understand your idea from your first answer. I'm happy to announce that we don't even need to make a compromise. You can stop a single test after fail without stop the other tests.

@SD10
Copy link
Member

SD10 commented Jul 8, 2017

@flashspys Yeah the method I showed above would work. What's happening is there is no test report being shown due to the crash. XCTAssertNotNil is still detecting the nilness of the String.

screen shot 2017-07-08 at 5 03 25 pm

Feel free to open a Pull Request making these changes to the tests where we use XCTAssertNotNil. In general, this is a solid example as to why we should stop using force unwrapping even in tests.

omaralbeik added a commit that referenced this issue Jul 20, 2017
- new rgbComponents extension in UIColor
- new docs from old docs repo
- imporved tests for UIColor
- Fixed #188
@omaralbeik omaralbeik mentioned this issue Jul 21, 2017
6 tasks
@omaralbeik
Copy link
Member

Fixed in v3.1.0

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

No branches or pull requests

3 participants