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

refactor test_message.py into seperate files #15661

Closed

Conversation

thedeveloperr
Copy link
Collaborator

@thedeveloperr thedeveloperr changed the title WIP refactor test_message.py into seperate files [WIP] refactor test_message.py into seperate files Jul 4, 2020
@thedeveloperr
Copy link
Collaborator Author

@timabbott @showell Please review if I am going in right direction.

@thedeveloperr
Copy link
Collaborator Author

rebased with #15675 as per @showell suggestion.

@thedeveloperr thedeveloperr changed the title [WIP] refactor test_message.py into seperate files refactor test_message.py into seperate files Jul 6, 2020
@thedeveloperr
Copy link
Collaborator Author

@timabbott @showell Ready for review. There are few other tests which can be split out but I am still figuring out on those. But for now I think these can be merged.

Starting with extracting out MirroredMessageUsersTests as it is related to
mirror users than anything message-specific.
In a future commit, may extract out some tests from MessagePOSTTest as well
but still deciding on those.
This commit extracts out MessagePOSTTest class from test_messages.py
intially.
In future commits other related message sending tests will be moved from
test_messages.py to test_message_send.py.
….py.

The commit moves, test_create_mirror_user_despite_race which is not related
to message sending from MessagePOSTTest class in test_message_send.py to
test_mirror_users.py.
This commit moves ScheduledMessageTest that tests sending scheduled
messages from test_messages.py to test_message_send.py.
StreamMessagesTest test stuff after message is sent to a stream, so
moving it out from test_messages.py to test_message_send.py.
This commit moves few tests related to testing proper sending of private
messages from PrivateMessagesTest class in test_messages.py to a new class
in test_message_send.py.
This commit moves ExtractTest class to test_message_send.py as they
test input parsing extract_* functions used in message sending enpoints.
This commit moves InternalPrepTest test class to test_message_send.py
because it tests internal_send_* and internal_prep_* functions which are
used for internal message sending in zulip.
MessageHydrationTest tests hydration done by MessageDict, so moving it
out of test_messages.py to test_message_dict.py.
This commit moves TestMessageForIdsDisplayRecipientFetching class which
have tests regarding display_recipient filled in by MessageDict to
test_message_dict.py.
This commit moves out the SoftDeactivationMessageTest out of
test_messages.py (which at the moment have mixed category of tests) into
a more logical file, test_soft_deactivation.py.
@timabbott
Copy link
Sponsor Member

These changes are great, thanks @thedeveloperr! I merged this PR with no changes as the series ending with 98cff4e. For next steps, I think we should:

  • Rename test_narrow.py to test_message_fetch.py
  • TestCrossRealmPMs and TestAddressee should go to test_message_send.py.
  • SewMessageAndReactionTest should go to test_message_dict.py.
  • test_unread.py should be renamed to test_message_flags.py
  • MessageAccessTests should be moved to test_message_flags.py
  • MessageHasKeywordsTest should be moved to test_message_fetch.py
  • MissedMessageTest should go to test_message_send.py
  • LogDictTest should be deleted along with the actual to_log_dict in models.py; that code has been unused for years.
  • CheckMessageTest should be in test_message_send.py
  • MessageVisibilityTest should be in test_message_fetch.py
  • TestBulkGetHuddleUserIds should probably be folded into the display recipient test class
  • NoRecipientIDsTest should probably be moved into test_subs.py alongside similar tests.

I believe that list would complete the removal of test_messages.py, with most of the content going to reasonably sized test files with names that align with the zerver/views/ filenames. Once that's done we can plan a follow-project to figure out how to split the new test_message_fetch.py, which will also be a giant at that point.

Thanks @thedeveloperr!

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