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

[Workspace] Add tests in executable template #1610

Merged
merged 1 commit into from Jun 19, 2018

Conversation

ankitspd
Copy link
Member

rdar://problem/36554558 Default executable SwiftPM package should have tests

@ankitspd
Copy link
Member Author

@swift-ci please smoke test

@ankitspd
Copy link
Member Author

@swift-ci please smoke test

<rdar://problem/36554558> Default executable SwiftPM package should have tests
@ankitspd
Copy link
Member Author

@swift-ci please smoke test

@ankitspd ankitspd requested a review from hartbit June 19, 2018 16:33
@ankitspd ankitspd merged commit fa16db4 into apple:master Jun 19, 2018
@ankitspd ankitspd deleted the exec-tests branch June 19, 2018 17:35
for bundle in Bundle.allBundles where bundle.bundlePath.hasSuffix(".xctest") {
return bundle.bundleURL.deletingLastPathComponent()
}
fatalError()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we add a comment to the fatalError? fatalError("Couldn't find the products directory")

Copy link
Collaborator

Choose a reason for hiding this comment

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

Why aren't we doing the same thing SwiftPMProduct is doing on Linux?

return AbsolutePath(CommandLine.arguments.first!, relativeTo: currentWorkingDirectory)
    .parentDirectory.appending(self.exec)

Copy link
Member Author

Choose a reason for hiding this comment

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

Most likely because it wasn't implemented at the time.

// Use XCTAssert and related functions to verify your tests produce the correct
// results.

guard #available(macOS 10.13, *) else {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think users who init this might be wondering why it's guarded with mac availability. I think we need to put a comment.

return
}

let fooBinary = productsDirectory.appendingPathComponent("foo")
Copy link
Collaborator

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 great to move the whole code that runs an executable into a function to encourage people to continue with that in other tests.

Copy link
Member Author

Choose a reason for hiding this comment

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

I explicitly don't want to do that because then its wrapping NSProcess and it would be difficult to create a correct wrapper while keeping the sample code small.

@ankitspd
Copy link
Member Author

Thanks @hartbit, addressed feedback here: #1612

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.

None yet

2 participants