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

Update XCTestSuite comment to Swift #100

Merged
merged 2 commits into from Apr 25, 2016
Merged

Update XCTestSuite comment to Swift #100

merged 2 commits into from Apr 25, 2016

Conversation

jonallured
Copy link
Contributor

This update also drops the parts describing features of Apple's implementation that are currently unavailable. Those parts are:

  • automatic test case extraction (testSuiteForTestCaseClass)
  • automatic creation of all test cases found in runtime (defaultTestSuite)

This PR is for https://bugs.swift.org/browse/SR-1145, hopefully I did this right! :D

This update also drops the parts of Apple's implementation that are
currently unavailable.
@modocache
Copy link
Collaborator

Excellent, thanks for taking this on, @jonallured! I have a few comments that I'll leave inline...

@@ -10,28 +10,15 @@
// XCTestSuite.swift
// A collection of test cases.
//
// A concrete subclass of XCTest, XCTestSuite is a collection of test cases.
// Suites are usually managed by the IDE, but XCTestSuite can also be
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is not true in the case of swift-corelibs-xctest. We build up a hierarchy of suites based on the test cases the user passes into XCTMain(). No IDE (Xcode, in the case of Apple XCTest) is involved.

@jonallured
Copy link
Contributor Author

Thanks for the feedback @modocache - I just pushed a commit that I think reads better. Any further thoughts?

@modocache
Copy link
Collaborator

Excellent, thanks! 💯

@modocache modocache merged commit 6913f61 into apple:master Apr 25, 2016
modocache added a commit that referenced this pull request Apr 25, 2016
The improved documentation from #100
used source comment syntax (`//`), not documentation syntax (`///`). It
also combined the header comment with the documentation for XCTestSuite.
@modocache
Copy link
Collaborator

Ack, sorry @jonallured -- I noticed immediately after merging that you used source-level comments (//), not documentation comments (///). I fixed this in 244507b. Thanks again for the documentation improvements!!

@jonallured
Copy link
Contributor Author

Hey @modocache, I noticed the two vs three slashes, but wasn't sure what their significance was - now I do! :D

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