zulip-terminal icon indicating copy to clipboard operation
zulip-terminal copied to clipboard

Published 20 hours ago • verified publisher icon zulip

WIP: tests: helper: Add tests for set_count.

Open kaustubh-nair opened this issue 5 years ago • 7 comments

  • [x] Test streams
  • [x] Test pms
  • [x] Test self-pms
  • [x] Test group pms
  • [x] ~~Test messages sent by user~~ ( set_count not called)
  • [x] Test muted streams
  • [x] Test muted topics
  • [ ] ~~Test topics whose view is open~~ ( UI test, fix in different PR )
  • [x] Test screen updated
  • [ ] Test mentions

kaustubh-nair avatar Mar 18 '20 09:03 kaustubh-nair

This is an important stepping stone to fixing #535. Handling read events will require modifications to set_count. Since, set_count is not being tested at all, I'm adding tests for the same. It might be difficult to cover every aspect of set_counts, but the goal is to test atleast the major features, so that significant bugs do not arise.

kaustubh-nair avatar Mar 18 '20 09:03 kaustubh-nair

There's will be a little bit of duplication across the tests for setting up and initializing variables. I'm wondering if it would be viable to abstract out the setup, or would it make the tests too complex?

kaustubh-nair avatar Mar 18 '20 14:03 kaustubh-nair

@neiljp @sumanthvrao, if you have time to give this a review please let me know if these tests look okay. :) And also the extent to which I should be testing set_count, so I can go ahead with the rest of the tests?

kaustubh-nair avatar Mar 22 '20 18:03 kaustubh-nair

This looks like a great start @kaustubh-nair . If possible we can try reusing some fixtures in conftest.py which may help us avoid the duplication (not entirely I suppose).

The list you have mentioned above seems comprehensive enough! Let's go ahead with that.

sumanthvrao avatar Mar 25 '20 04:03 sumanthvrao

The fixtures in conftest.py don't contain enough messages to properly test the counts, so I ended up adding new ones

kaustubh-nair avatar Mar 25 '20 05:03 kaustubh-nair

@sumanthvrao @neiljp This is ready for an initial review. I do realize there is a LOT of duplication going on here. I will push some refactor commits sometime later tomorrow.

kaustubh-nair avatar Mar 30 '20 13:03 kaustubh-nair

Heads up @kaustubh-nair, we just merged some commits that conflict with the changes your made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the upstream/master branch and resolve your pull request's merge conflicts accordingly.

zulipbot avatar May 13 '20 08:05 zulipbot