Skip to content

[stdlib] Fix and Improve Unit Tests for Set #24575

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 3 commits into from
May 9, 2019
Merged

[stdlib] Fix and Improve Unit Tests for Set #24575

merged 3 commits into from
May 9, 2019

Conversation

@palimondo
Copy link
Contributor

@palimondo palimondo commented May 7, 2019 •
edited
Loading

Analyzing how well the #21300 handles isEmpty cases, I've noticed that the unit test that should be testing Set.Sequence variants of methods:

  • isSubset
  • isSuperSet
  • isStrictSubset
  • isStrictSuperSet

contained errors (sometimes testing Set.Set variants) and the tests did not fully cover the potential edge cases which might arise when implementing performance optimizations for the empty cases.

This PR fixes these errors and adds equivalent coverage for both Set.Set and Set.Sequence variants.

palimondo added 2 commits May 7, 2019 19:35
@palimondo
[stdlib] Improve unit test coverage of Set.Set
475c209 
Systematically test also set algebra:
* with empty sets on both sides
* inverse operation
@palimondo
[stdlib] Fix unit tests for Set.Sequence methods
0f4c1cd 
Really test the Sequence variants of set algebra and cover the same cases as Set.Set.
@palimondo
Copy link
Contributor Author

palimondo commented May 7, 2019

@swift-ci please test

@palimondo palimondo requested a review from dabrahams May 7, 2019 22:31
lorentey
lorentey approved these changes May 8, 2019
View reviewed changes
Copy link
Member

@lorentey lorentey left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Nice catch! These are good additions; I only have the small nit below.

palimondo
palimondo commented May 9, 2019
View reviewed changes
@palimondo
[stdlib] Set.Sequence unittest explicit annotation
bce023a 
Use explicit type annotation to make it crystal clear (beyoud any reasonable doubt) these are indeed testing the Sequence variants.
@palimondo
Copy link
Contributor Author

palimondo commented May 9, 2019

@swift-ci please test

@palimondo palimondo merged commit 14f250b into swiftlang:master May 9, 2019
@palimondo palimondo deleted the set-seq-unittests branch May 9, 2019 14:39
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

@airspeedswift airspeedswift Awaiting requested review from airspeedswift

@eeckstein eeckstein Awaiting requested review from eeckstein

@dabrahams dabrahams Awaiting requested review from dabrahams

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@palimondo @lorentey