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

Comments

Projects
None yet
3 participants
@flashspys
Copy link
Contributor

flashspys commented Jul 8, 2017

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

This comment has been minimized.

Copy link
Contributor Author

flashspys commented Jul 8, 2017

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

XCTest.assertOptionalNotNil

@SD10 SD10 added the possible bug label Jul 8, 2017

@SD10

This comment has been minimized.

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

This comment has been minimized.

Copy link
Contributor Author

flashspys commented Jul 8, 2017

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

@SD10

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

Copy link
Contributor Author

flashspys commented Jul 8, 2017

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

This comment has been minimized.

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

Improved UIColor tests + more docs + #188
- new rgbComponents extension in UIColor
- new docs from old docs repo
- imporved tests for UIColor
- Fixed #188

@omaralbeik omaralbeik referenced this issue Jul 21, 2017

Merged

3.1.0 #208

6 of 6 tasks complete

@omaralbeik omaralbeik closed this Jul 23, 2017

@omaralbeik

This comment has been minimized.

Copy link
Member

omaralbeik commented Jul 23, 2017

Fixed in v3.1.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.