zulip-terminal
zulip-terminal copied to clipboard
Split test_ui_tools to multiple smaller test files.
The reason for this change is to
- make it wasy to find position to add new tests (Especially for new contributors)
- To make the test structure similar to code structure
@amanagr @neiljp A word of caution that if we decide to go ahead with this approach, this would cause conflicts with most of the PRs we have on going.
@sumanthvrao Zulip server has used a tool to work with that issue, which I think is TinglingGit
, though I've not used it and it may be overkill here. We should possibly just try and review/merge some more first, though that tool may indicate which would be good to reduce conflicts on merging this.
Great! I'm looking into it.
Here's an output of running the tool for tests/ui/test_ui_tools.py
:
tests/ui/test_ui_tools.py
[PR#387(+7-7)](https://github.com/zulip/zulip-terminal/pull/387)
[PR#400(+4-46)](https://github.com/zulip/zulip-terminal/pull/400)
[PR#278(+46-0)](https://github.com/zulip/zulip-terminal/pull/278)
[PR#419(+0-5)](https://github.com/zulip/zulip-terminal/pull/419)
[PR#315(+145-1)](https://github.com/zulip/zulip-terminal/pull/315)
[PR#335(+6-2)](https://github.com/zulip/zulip-terminal/pull/335)
[PR#354(+129-2)](https://github.com/zulip/zulip-terminal/pull/354)
[PR#358(+36-1)](https://github.com/zulip/zulip-terminal/pull/358)
[PR#367(+34-0)](https://github.com/zulip/zulip-terminal/pull/367)
[PR#382(+211-5)](https://github.com/zulip/zulip-terminal/pull/382)
Around 10 open PRs have changes involving some parts of test_ui_tools.py
@sumanthvrao How are the conflicts looking like now?
@neiljp I believe they are still the same amount of conflicts which would probably appear if we got merged.
Heads up @sumanthvrao, 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.
This was a useful investigation :+1:
However, this has been open a long time, and we made this transition initially starting some tests in separate files, and moving them across in fa2fbedcc for buttons, and d5a22ba2b167 for messages (into a distinct test_messages.py).
It would likely still be useful to make changes such as:
- move view element tests (eg. StreamsView, MessagesView) from ui/test_ui_tools into ui_tools/test_views (matching existing code positions)
- split popup code out from ui_tools/views into ui_tools/popups (matching existing test split)
I've filed the latter suggestions as #1508 so this isn't lost.