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

Consistently use self.assert_length in tests. #18522

Merged
merged 3 commits into from May 19, 2021

Conversation

abhijeetbodas2001
Copy link
Member

Changes made with Find&Replace with regex.

@abhijeetbodas2001 abhijeetbodas2001 changed the title Consistently user self.assert_length in tests. Consistently use self.assert_length in tests. May 17, 2021
@@ -864,14 +864,14 @@ def assert_json_error(self, result: HttpResponse, msg: str, status_code: int = 4
"""
self.assertEqual(self.get_json_error(result, status_code=status_code), msg)

def assert_length(self, items: List[Any], count: int) -> None:
def assert_length(self, items: Any, count: int) -> None:
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

I think you want the appropriate abstract base class, Sized, here.

Copy link
Sponsor Member

Choose a reason for hiding this comment

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

Probably Collection, actually, since we do iterate through it.

@timabbott
Copy link
Sponsor Member

This looks great aside from the type-checking detail there: We never want to use Any, especially when an appropriate abstract class like Collection is appropriate, since Any effectively disables type-checking.

Make it so that `assert_length` can be used for not
just lists but all `Collections`.

This is prep for using this helper consistently for
all tests.
This helper does some nice things like printing out
the data structure incase of failure.
@abhijeetbodas2001
Copy link
Member Author

Thanks, fixed to use Collection now.

@timabbott timabbott merged commit 0ec905e into zulip:master May 19, 2021
@timabbott
Copy link
Sponsor Member

Merged, thanks @abhijeetbodas2001!

@abhijeetbodas2001 abhijeetbodas2001 deleted the assert_length branch May 20, 2021 04:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants