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 UI Testing #55

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Support UI Testing #55

wants to merge 6 commits into from

Conversation

timshadel
Copy link

Add support for easily faking network requests in your app during UI testing.

This so you can pass a cassette URL to your app in `launchEnvironment` that points to a resource in your UI testing target.
@timshadel timshadel force-pushed the ui-testing branch 10 times, most recently from 9ef6de2 to dbc2704 Compare August 12, 2016 19:02
This also catches and fixes the test failure on master involving a file
that isn't being created because the directory didn't exist.
@brentleyjones
Copy link

This would be a great feature to have.

Copy link
Contributor

@soffes soffes left a comment

Choose a reason for hiding this comment

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

This looks great. A few minor changes, but I think this will be a great improvement!

@@ -195,6 +197,10 @@ public class Session: NSURLSession {
}
}

var cassetteName = "cassette"
if let s = cassetteURL?.lastPathComponent {
cassetteName = s.substringToIndex(s.endIndex.advancedBy(-5))
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be much better to get the length directly instead of guessing it's 5

@@ -214,9 +220,9 @@ public class Session: NSURLSession {
if let data = string.dataUsingEncoding(NSUTF8StringEncoding) {
data.writeToFile(outputPath, atomically: true)
print("[DVR] Persisted cassette at \(outputPath). Please add this file to your test target")
} else {
print("[DVR] Failed to persist cassette.")
Copy link
Contributor

Choose a reason for hiding this comment

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

In my PR to convert to Swift, I added a return on 223 instead. Slightly more concise :)

lazy var testFile: NSURL = {
return NSBundle(forClass: self.dynamicType).URLForResource("testfile", withExtension: "txt")!
lazy var testFile: NSURL? = {
return NSBundle(forClass: self.dynamicType).URLForResource("testfile", withExtension: "txt")
Copy link
Contributor

Choose a reason for hiding this comment

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

👎 In tests, it's better to ! things so you can see the error right away durning the test.

XCTFail("Error uploading file: \(error)")
return
}
guard let data = data else { XCTFail("Missing request data"); return }
Copy link
Contributor

Choose a reason for hiding this comment

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

👎 No sense in doing this. The test will fail if it's nil anyway.

@@ -87,6 +96,12 @@ class SessionUploadTests: XCTestCase {
func writeDataToFile(data: NSData, fileName: String) -> NSURL {
let documentsPath = NSSearchPathForDirectoriesInDomains(.DocumentDirectory, .UserDomainMask, true)[0]
let documentsURL = NSURL(fileURLWithPath: documentsPath, isDirectory: true)

do {
try NSFileManager.defaultManager().createDirectoryAtURL(documentsURL, withIntermediateDirectories: true, attributes: nil)
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here: try!

Copy link
Author

Choose a reason for hiding this comment

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

Actually all the !s were what was keeping you guys from having passing tests on Travis. I had to go through and remove them all to actually see what was truly failing and this piece here was the crux of the fix. The mac tests would pass and the iOS tests would fail because this file was expected but didn't exist. Travis (xcodebuild) only reports:

      -[SessionUploadTests testUploadFile]
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
fatal error: unexpectedly found nil while unwrapping an Optional value
Child process terminated with signal 4: Illegal instruction
Test crashed while running.

Since there's no indication of where this crashed, and the test passed in Xcode itself, the actual error took like ½ a day to track down and was super annoying. So, while I like the theory that !s are awesome in testing, this bug totally proves them to still be harmful.

Copy link
Author

Choose a reason for hiding this comment

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

@soffes Do you want me to put all the !s back anyway?

class LoginUITests: XCTestCase {
func testUseValidAuth() {
let app = XCUIApplication()
app.launchEnvironment["cassette"] = NSBundle(forClass: LoginUITests.self).URLForResource("valid-auth", withExtension: "json")!.absoluteString
Copy link
Contributor

Choose a reason for hiding this comment

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

✨ so cool!

@TheMetalCode
Copy link

TheMetalCode commented Jun 20, 2018

So I'm to the point of wanting to use DVR to stub out network calls for XCTest UI scenarios. Is there more to do with this PR to make it mergeable? Can I help with any of that? Or have there been further developments since this PR opened that would make one able to use DVR with XCTest UI?

No pressure, I know OSS is a volunteer effort, just looking to help.

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