Skip to content

[stdlib] Addressing Fixme comment atomic stdlib unit test flags #23198

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 12 commits into from
Mar 28, 2019
Merged

[stdlib] Addressing Fixme comment atomic stdlib unit test flags #23198

merged 12 commits into from
Mar 28, 2019

Conversation

LucianoPAlmeida
Copy link
Contributor

Taking a shot at the FIXME comment in StdlibUnittest to make some flags atomic.

Does not solve any SR.

@LucianoPAlmeida
Copy link
Contributor Author

Hey :)) Just pinging you stdlib guys for quick a feedback on this one
cc @natecook1000 @lorentey @airspeedswift

@lorentey
Copy link
Member

@swift-ci please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - 2ba4516

@LucianoPAlmeida
Copy link
Contributor Author

Hey guys :)
This MacOS build is failing, but actually I couldn't find out the error on the logs. Just run the local test with ./utils/build-script --test --clean on the Mac and seems fine. Notice there are other PRs failing there too, any chance of being something with the CI?

lorentey
lorentey requested changes Mar 22, 2019
View reviewed changes
Copy link
Member

@lorentey lorentey left a comment

Choose a reason for hiding this comment

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

I have a suggestion to improve readability; and to add a note about a potential issue -- otherwise it looks good!

@lorentey
Copy link
Member

@LucianoPAlmeida These failures sometimes occur if master happens to be temporarily broken. Unfortunately the build logs aren't available -- I'll spawn a new test just to be sure.

@swift-ci please test macOS platform

lorentey
lorentey reviewed Mar 26, 2019
View reviewed changes
stdlib/private/StdlibUnittest/StdlibUnittest.swift Outdated

/// Run `body` and expect a failure to happen.
///
/// The check passes iff `body` triggers one or more failures.
Copy link
Member

Choose a reason for hiding this comment

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

This iff wasn't a typo -- it's a shorthand for "if and only if".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, sorry 😅

@lorentey
Copy link
Member

@swift-ci test

@swift-ci

This comment has been minimized.

Sign in to view
@swift-ci

This comment has been minimized.

Sign in to view
lorentey
lorentey requested changes Mar 26, 2019
View reviewed changes
@lorentey
Copy link
Member

@swift-ci test

@swift-ci

This comment has been minimized.

Sign in to view
@lorentey
Copy link
Member

@swift-ci test

@swift-ci

This comment has been minimized.

Sign in to view
@swift-ci

This comment has been minimized.

Sign in to view
@lorentey lorentey merged commit a62c87f into swiftlang:master Mar 28, 2019
@LucianoPAlmeida LucianoPAlmeida deleted the fixme-atomic-stdlib-unit-test branch March 29, 2019 01:49
@LucianoPAlmeida
Copy link
Contributor Author

Thanks @lorentey :))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lorentey lorentey lorentey approved these changes

@moiseev moiseev Awaiting requested review from moiseev

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@LucianoPAlmeida @lorentey @swift-ci