tests: boxes: Add tests for saving drafts.

[Niloth-p]

What does this PR do, and why?

Add 2 test functions:

1. Draft message to a channel
2. Draft message to a private narrow (DM and PM huddle)

Test for an existing saved draft for each of the test cases.

Test for:

• a function call to display the save-draft-confirmation popup, in case of existing saved draft
• the created draft object
• return of the key without creating a draft, in case of invalid recipients

This does not test the _tidy_valid_recipients_and_notify_invalid_ones or the session_draft_message functions. They're mocked. Tests for them would need to be added separately.

External discussion & connections

• [ ] Discussed in #zulip-terminal in topic
• [ ] Fully fixes #
• [ ] Partially fixes issue #
• [x] Builds upon previous unmerged work in PR #950
• [ ] Is a follow-up to work in PR #
• [ ] Requires merge of PR #
• [ ] Merge will enable work on #

How did you test this?

• [ ] Manually - Behavioral changes
• [ ] Manually - Visual changes
• [ ] Adapting existing automated tests
• [ ] Adding automated tests for new behavior (or missing tests)
• [ ] Existing automated tests should already cover this (only a refactor of tested code)

Self-review checklist for each commit

• [x] It is a minimal coherent idea
• [x] It has a commit summary following the documented style (title & body)
• [ ] It has a commit summary describing the motivation and reasoning for the change
• [x] It individually passes linting and tests
• [ ] It contains test additions for any new behavior
• [ ] It flows clearly from a previous branch commit, and/or prepares for the next commit

[Niloth-p]

Updated to add the mocked_saved_drafts list. Took that outside both the test functions to reduce redundancy. Also switched from using 'huddle' to 'dm_group' in the ids and message.

[rsashank]

LGTM! Great work @Niloth-p 👍

I'm trying to see if we can simplify the conditionals in the test by adding more to the parameterize, but I'm not sure.

[Niloth-p]

Thank you for catching those details, @rsashank! Great review, that was very helpful. Updated the requested changes.

[zormit]

@Niloth-p I have tried simplifying and typing your code a bit in https://github.com/zormit/zulip-terminal/tree/1495-save-draft/pr (713bf505, in case I change the branch). What do you think?

I'm still not fully happy with all the layers of indirection, probably it can be simplified even more, but this might give you an idea. I have to stop for now, so I'm sending you this idea, please take it if you like it. (edit: I realized tests are not even all passing, but that seems fixable :))

[Niloth-p]

@zormit Thank you lots for making those changes yourself! That does look much better.

Sorry for the delay in replying back, I'd absorbed your commits right away, but hadn't managed to type out a reply here.

If you find this interesting, please feel free to push your own PR with other improvements and add me as a co-author, that could help get it merged too, as I've done several iterations on this previously, and have been struggling to identify good code and bad code here.

So, no, I'm not really clear on how we can further reduce the indirection, my apologies. I can think of several ways to change the tests, but I have no idea what improves it and what worsens it. If someone is able to guide me with 1 liners like the commit messages, I'll try my best to implement it and get back.

Being new to the complexities of testing, when I come across best practices and guidance, I'm not sure to what extent I should be following them, that I realise I might have overdone it here to the point of complexity, and I'm still not clear enough on all the tradeoffs, so please do let me know how we can proceed from here.

Also, I'm not sure how many commits this is supposed to be. I was initially under the impression this should be a single commit, since it's just tests, and we wouldn't want to be tracking the additions incrementally. But, after the suggestion from @neiljp to break down the refactoring commits of lint-hotkeys, I've been wondering whether we should break this down too. Do we need to?

I've added some newer test cases. But I'm not sure if we want to be that thorough or if it bulks up the testing time unnecessarily.

[Niloth-p]

Github will report the tests as failed, because I've kept my addition separate from Moritz's commits, but they'll pass if they're squashed. So, please ignore them.