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

Split test_ui_tools to multiple smaller test files.

Open sumanthvrao opened this issue 5 years ago • 7 comments

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

sumanthvrao avatar Jul 08 '19 11:07 sumanthvrao

@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 avatar Jul 08 '19 11:07 sumanthvrao

@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.

neiljp avatar Jul 08 '19 14:07 neiljp

Great! I'm looking into it.

sumanthvrao avatar Jul 08 '19 14:07 sumanthvrao

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 avatar Jul 08 '19 16:07 sumanthvrao

@sumanthvrao How are the conflicts looking like now?

neiljp avatar Jul 28 '19 22:07 neiljp

@neiljp I believe they are still the same amount of conflicts which would probably appear if we got merged.

sumanthvrao avatar Jul 29 '19 00:07 sumanthvrao

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.

zulipbot avatar Apr 17 '20 00:04 zulipbot

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.

neiljp avatar May 30 '24 05:05 neiljp