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

TRACKING: Migrate tests using hard-coded keys to use keys_for_command. #533

Closed
1 of 2 tasks
preetmishra opened this issue Mar 10, 2020 · 6 comments
Closed
1 of 2 tasks
Labels
area: tests further discussion required Discuss this on #zulip-terminal on chat.zulip.org
Milestone

Comments

@preetmishra
Copy link
Member

preetmishra commented Mar 10, 2020

@neiljp In #523, we migrated keypress tests to use keys_for_command but there are still few other tests that use hard-coded keys. This issue serves the purpose of tracking that.

I grepped the code with keypress to see what tests still use hard-coded keys. I am listing out those here.

  • While we migrated a bunch of keypress tests in tests: Migrate keypress tests to use keys_for_command. #523, there are a few which still use hard-coded keys.
  • All the mouse_event tests use hard-coded keys. It is not possible to directly use keys_for_command here as their implementation doesn't support all the keys for the commands (more information about this is in the comments).

If you find more tests using hard-coded keys, you can append them here.

@preetmishra
Copy link
Member Author

The mouse_event implementations only support up and down (example) but the keys_for_command for GO_UP/DOWN also have k/j.

@preetmishra
Copy link
Member Author

@zulipbot add "tests (pytest)" "further discussion required"

@zulipbot zulipbot added further discussion required Discuss this on #zulip-terminal on chat.zulip.org area: tests labels Mar 10, 2020
@neiljp
Copy link
Collaborator

neiljp commented Mar 10, 2020

@preetmishra The mouse_event code is a little special, as most typically are delegating to the keypress methods, eg. it mimics GO_UP or GO_DOWN.

It is possible that we (or a user) might remove the up or down keys from keys.py, so we should probably rectify the code to use one valid key from the keys instead, which we can then test.

@preetmishra
Copy link
Member Author

@neiljp I have proposed an implementation in #539 that uses keys_for_command to solve this.

zee-bit added a commit to zee-bit/zulip-terminal that referenced this issue Apr 1, 2021
This commit migrates mouse_event functions to use primary_key_for_commands
instead of hard-coded keys to ensure that they are resistant to
breakage if navigational keys are updated in future.

Fixes zulip#469 and zulip#533 partially.
@zee-bit
Copy link
Member

zee-bit commented Apr 1, 2021

The mouse_event implementations only support up and down (example) but the keys_for_command for GO_UP/DOWN also have k/j

@preetmishra We could use primary_key_for_command instead; that will only give the up and down keys (or primary ones).
I have a fix for this issue as #953(one of the commits is borrowed from your #539 😅 :).

zee-bit added a commit to zee-bit/zulip-terminal that referenced this issue May 17, 2021
This commit migrates mouse_event functions to use primary_key_for_commands
instead of hard-coded keys to ensure that they are resistant to
breakage if navigational keys are updated in future.

Fixes zulip#469 and zulip#533 partially.
zee-bit added a commit to zee-bit/zulip-terminal that referenced this issue Jun 2, 2021
This commit migrates mouse_event functions to use primary_key_for_commands
instead of hard-coded keys to ensure that they are resistant to
breakage if navigational keys are updated in future.

Fixes zulip#469 and zulip#533 partially.
zee-bit added a commit to zee-bit/zulip-terminal that referenced this issue Jun 25, 2021
This commit migrates mouse_event functions to use primary_key_for_commands
instead of hard-coded keys to ensure that they are resistant to
breakage if navigational keys are updated in future.

Tests amended.

Fixes zulip#469 and zulip#533.
zee-bit added a commit to zee-bit/zulip-terminal that referenced this issue Jul 4, 2021
This commit migrates mouse_event functions to use primary_key_for_commands
instead of hard-coded keys to ensure that they are resistant to
breakage if navigational keys are updated in future.

Tests amended.

Fixes zulip#469 and zulip#533.
zee-bit added a commit to zee-bit/zulip-terminal that referenced this issue Jul 7, 2021
This commit migrates mouse_event functions to use primary_key_for_commands
instead of hard-coded keys to ensure that they are resistant to breakage
if navigational keys are updated in future.

Tests amended.

Fixes zulip#469 and zulip#533.
zee-bit added a commit to zee-bit/zulip-terminal that referenced this issue Jul 8, 2021
This commit migrates mouse_event functions to use primary_key_for_commands
instead of hard-coded keys to ensure that they are resistant to breakage
if navigational keys are updated in future.

Tests amended.

Fixes zulip#469 and zulip#533.
@neiljp neiljp added this to the Next Release milestone Jul 15, 2021
@neiljp
Copy link
Collaborator

neiljp commented Jul 15, 2021

Closing this after #953, though we have a very few use of isolated keys (navigation and activation) which will likely be solved using command-map work.

@neiljp neiljp closed this as completed Jul 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: tests further discussion required Discuss this on #zulip-terminal on chat.zulip.org
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants