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

Collection matchers #350

Closed
wants to merge 4 commits into from
Closed

Collection matchers #350

wants to merge 4 commits into from

Conversation

bpjatfb
Copy link
Contributor

@bpjatfb bpjatfb commented Mar 27, 2023

What:

Add Collection matchers, as requested in #346

  • AnyContaining(element): A container that contains element
  • AnyContainingAll(element_list): A container that contains every element of element_list
  • AnyIterable(): Any iterable
  • IterableWithElements(element_list): an iterable containing all the elements in element_list in the same order"
  • AnyNotEmpty(): An object where len() does not evaluate to zero"
  • AnyEmpty(): An object where len() evaluates to zero"

Why:

Currently, the official matchers support Lists and Dicts, but not much in the way of abstract collections. This PR enables patterns like:

actual = deque([1, 2, 3], maxlen=100)
self.assertEqual(actual, IterableWithElements([1, 2, 3]))

How:

I added these matchers to matchers.py alongside their List/Dict counterparts. I added unittests to matchers_unittest.py. Finally, I added documentation in patching/argument_matchers/.

Risks:

No risk of breaking existing code, since these are largely standalone, however there are some edge cases for these matchers that may or may not be intuitive to the user.

  • AnyEmpty/AnyNotEmpty rely on len(), so they will throw on AnyEmpty() == iter([1, 2, 3]), for example.
  • AnyContaining()/AnyContainingAll() rely on in, so will try to match anything that implements __contains__
    • They may also exhaust iterators leading to unexpected results.

Checklist:

  • Added tests, if you've added code that should be tested
  • Updated the documentation, if you've changed APIs
  • Ensured the test suite passes
  • Made sure your code lints
  • Completed the Contributor License Agreement ("CLA")

@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Mar 27, 2023
@coveralls
Copy link

Pull Request Test Coverage Report for Build 4537294758

  • 34 of 37 (91.89%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.03%) to 93.728%

Changes Missing Coverage Covered Lines Changed/Added Lines %
testslide/matchers.py 34 37 91.89%
Totals Coverage Status
Change from base Build 4472249756: -0.03%
Covered Lines: 2660
Relevant Lines: 2838

💛 - Coveralls

@facebook-github-bot
Copy link

@deathowl has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@deathowl
Copy link
Member

Very nice, imported, feel free to accept ship once internal tests finish

@bpjatfb
Copy link
Contributor Author

bpjatfb commented Mar 28, 2023

Awesome, thanks @deathowl!

@facebook-github-bot
Copy link

@deathowl merged this pull request in 798c9e8.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Do not delete this pull request or issue due to inactivity. Merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants