Skip to content

[stdlib] Fix UnsafeBufferPointer range subscript getter tests #3233

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

Merged
merged 1 commit into from
Jun 29, 2016

Conversation

austinzheng
Copy link
Contributor

What's in this pull request?

Fixing an issue with a unit test that caused its four variations to not all be run. Sorry! @gribozavr @moiseev

OS X tests pass.


Before merging this pull request to apple/swift repository:

  • Test pull request on Swift continuous integration.

Triggering Swift CI

The swift-ci is triggered by writing a comment on this PR addressed to the GitHub user @swift-ci. Different tests will run depending on the specific comment that you use. The currently available comments are:

Smoke Testing

Platform Comment
All supported platforms @swift-ci Please smoke test
All supported platforms @swift-ci Please smoke test and merge
OS X platform @swift-ci Please smoke test OS X platform
Linux platform @swift-ci Please smoke test Linux platform

Validation Testing

Platform Comment
All supported platforms @swift-ci Please test
All supported platforms @swift-ci Please test and merge
OS X platform @swift-ci Please test OS X platform
OS X platform @swift-ci Please benchmark
Linux platform @swift-ci Please test Linux platform

Lint Testing

Language Comment
Python @swift-ci Please Python lint

Note: Only members of the Apple organization can trigger swift-ci.

All four of the Unsafe*BufferPointer range subscript getter tests (one for each range) were using the same test name string. This caused only one of the four tests to actually be registered to the test suite. This change fixes that issue, and adds a couple more tests for better coverage.

@austinzheng austinzheng force-pushed the az-tests1 branch 2 times, most recently from 150e392 to d1d1cd4 Compare June 28, 2016 00:15
@austinzheng austinzheng changed the title Fix UnsafeBufferPointer range subscript getter tests [stdlib] Fix UnsafeBufferPointer range subscript getter tests Jun 28, 2016
All four of the Unsafe*BufferPointer range subscript getter tests (one for each range) were using the same test name string. This caused only one of the four tests to actually be registered to the test suite. This change fixes that issue, and adds a couple more tests for better coverage.
@austinzheng
Copy link
Contributor Author

@gribozavr I'm not in any rush to get this PR merged, but if it is merged I have a follow-up PR with more tests that depends on this one. (I'm also still working on the hashed collections PR.)

@moiseev
Copy link
Contributor

moiseev commented Jun 29, 2016

@swift-ci Please test

@gribozavr
Copy link
Contributor

LGTM.

@@ -15,6 +15,8 @@ struct SubscriptGetTest {
/// The values that should be expected by slicing the UBP, or `nil` if the
/// test is expected to crash.
let expectedValues: [Int]?
/// Same as `expectedValues`, but for closed ranges. `nil` if no difference.
let expectedClosedValues: [Int]?
Copy link
Contributor

Choose a reason for hiding this comment

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

There is let subscriptRangeTests in StdlibCollectionUnittest, which is very similar in intent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Those tests don't seem to be instrumented for use with the different types of ranges. Would a follow-up item to fix that be of interest to you?

Copy link
Contributor

Choose a reason for hiding this comment

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

Absolutely, having more powerful shared datasets would make all tests stronger.

@moiseev moiseev merged commit 26ade95 into swiftlang:master Jun 29, 2016
@austinzheng austinzheng deleted the az-tests1 branch June 29, 2016 21:59
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.

3 participants