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.

Sign up for GitHub

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

Jump to bottom

Migrate keypress tests to use keys_for_command #469

Closed
neiljp opened this issue Aug 31, 2019 · 7 comments · Fixed by #953
Closed

Migrate keypress tests to use keys_for_command #469

neiljp opened this issue Aug 31, 2019 · 7 comments · Fixed by #953
Labels
area: tests
Milestone
0.7.0

Comments

@neiljp
Copy link
Collaborator

neiljp commented Aug 31, 2019

Current keypress tests use hardcoded tests against specific keys. It would seem cleaner to use keys_for_command to both ensure this works for multiple specified keys, and also to automatically update if the standard keys are updated in config/keys.py.

This might be able to be simplified by writing an appropriate parametrized fixture.
@neiljp neiljp mentioned this issue Mar 4, 2020
Closed
@preetmishra
Copy link
Member

@zulipbot claim

preetmishra added a commit to preetmishra/zulip-terminal that referenced this issue Mar 5, 2020
@preetmishra
tests: ui/ui_tools: Migrate keypress tests to use keys_for_command.
91d896a 
The previous keypress tests used hardcoded tests against specific
keys. This could have potentially broken tests if any keybinding
would have updated.

Updated keypress tests to use `keys_for_command` to ensure the
tests are future-proof and they work for multiple  specified keys.

Fixes zulip#469.
preetmishra added a commit to preetmishra/zulip-terminal that referenced this issue Mar 5, 2020
@preetmishra
tests: ui/ui_tools: Migrate keypress tests to use keys_for_command.
537e50c 
The previous keypress tests used hardcoded tests against specific
keys. This could have potentially broken tests if any keybinding
would have updated.

Updated keypress tests to use `keys_for_command` to ensure the
tests are future-proof and they work for multiple specified keys.

Fixes zulip#469.
preetmishra added a commit to preetmishra/zulip-terminal that referenced this issue Mar 5, 2020
@preetmishra
tests: ui/ui_tools: Migrate keypress tests to use keys_for_command.
0af51d0 
The previous keypress tests used hardcoded tests against specific
keys. This could have potentially broken tests if any keybinding
would have updated.

Updated keypress tests to use `keys_for_command` to ensure the
tests are future-proof and they work for multiple specified keys.

Fixes zulip#469.
preetmishra added a commit to preetmishra/zulip-terminal that referenced this issue Mar 5, 2020
@preetmishra
tests: ui/ui_tools: Migrate keypress tests to use keys_for_command.
989d608 
The previous keypress tests used hardcoded tests against specific
keys. This could have potentially broken tests if any keybinding
would have updated.

Updated keypress tests to use `keys_for_command` to ensure that
the tests are future-proof and they work for multiple specified keys.

Fixes zulip#469.
preetmishra added a commit to preetmishra/zulip-terminal that referenced this issue Mar 5, 2020
@preetmishra
tests: ui/ui_tools: Migrate keypress tests to use keys_for_command.
01038b9 
The previous keypress tests used hardcoded tests against specific
keys. This could have potentially broken tests if any keybinding
would have updated.

Updated keypress tests to use `keys_for_command` to ensure that
the tests are future-proof and they work for multiple specified keys.

Fixes zulip#469.
preetmishra added a commit to preetmishra/zulip-terminal that referenced this issue Mar 6, 2020
@preetmishra
tests: ui/ui_tools: Migrate keypress tests to use keys_for_command.
819c05e 
The previous keypress tests used hardcoded tests against specific
keys. This could have potentially broken tests if any keybinding
would have updated.

Updated keypress tests to use `keys_for_command` to ensure that
the tests are future-proof and they work for multiple specified keys.

Fixes zulip#469.
preetmishra added a commit to preetmishra/zulip-terminal that referenced this issue Mar 6, 2020
@preetmishra
tests: ui/ui_tools: Migrate keypress tests to use keys_for_command.
3e8b447 
The previous keypress tests used hardcoded tests against specific keys.
This could have potentially broken tests if any keybinding would have
updated.

Updated trivial keypress tests to use `keys_for_command` to ensure that the
tests are future-proof and they work for multiple specified keys.

Fixes zulip#469.
@preetmishra preetmishra mentioned this issue Mar 6, 2020
Closed
preetmishra added a commit to preetmishra/zulip-terminal that referenced this issue Mar 6, 2020
@preetmishra
tests: ui/ui_tools: Migrate keypress tests to use keys_for_command.
faa96b4 
The previous keypress tests used hardcoded tests against specific keys.
This could have potentially broken tests if any keybinding would have
updated.

Updated trivial keypress tests to use `keys_for_command` to ensure that the
tests are future-proof and they work for multiple specified keys.

Fixes zulip#469.
@neiljp
Copy link
Collaborator Author

neiljp commented Mar 10, 2020 •
edited

Thanks to the work of @preetmishra in #523 this issue has been partially resolved 👍

@neiljp neiljp closed this as completed Mar 10, 2020
@neiljp neiljp reopened this Mar 10, 2020
@zulipbot
Copy link
Member

zulipbot commented Mar 20, 2020 •
edited

Hello @preetmishra, you have been unassigned from this issue because you have not updated this issue or any referenced pull requests for over 14 days.

You can reclaim this issue or claim any other issue by commenting @zulipbot claim on that issue.

Thanks for your contributions, and hope to see you again soon!

@zee-bit
Copy link
Member

zee-bit commented Mar 17, 2021

@neiljp any particular reason why it's still partially resolved?
A quick git grep indicates we have covered almost all cases of hardcoded keys in keypress events.

@neiljp
Copy link
Collaborator Author

neiljp commented Mar 17, 2021

I'm not entirely sure originally why this was left open. However, there are still explicit checks and/or calls to keypress with specific characters - that's not necessarily test-specific, though things (including tests) may break if we customized the keys.

@zee-bit
Copy link
Member

zee-bit commented Mar 17, 2021

Right. There are few instances of hard-coded keys that are not specific to tests, do you want me to fix those?
I have a small UI improvement PR that I was about to propose. I can push this refactor into that PR -- though, this may not be a good practice.
Or, I can make a new PR, with a few more reactors if I could find?

@neiljp
Copy link
Collaborator Author

neiljp commented Mar 17, 2021

Generally we want all the keys to be flexible to allow for customization, so explicitly calling with particular keys is quite likely to break at some point.

A separate PR is fine - it can often be quicker and easier to review, as long as it's straightforward. If there is another commit then they all need to be considered.

@zee-bit zee-bit mentioned this issue Mar 18, 2021
Merged
zee-bit added a commit to zee-bit/zulip-terminal that referenced this issue Apr 1, 2021
@zee-bit
refactor: views: Migrate mouse_events to use primary_key_for_command.
daebb05 
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 May 17, 2021
@zee-bit
refactor: views: Migrate mouse_events to use primary_key_for_command.
0375e6c 
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
@zee-bit
refactor: views: Migrate mouse_events to use primary_key_for_command.
3b5a47e 
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
@zee-bit
refactor: views: Migrate mouse_events to use primary_key_for_command.
9dae640 
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 Jun 25, 2021
@zee-bit
refactor: ui/views/boxes: Replace hardcoded key in keypress to commands.
862a18a 
This commit replaces all instances of hard-coded keys passed to keypress
that posed a risk of breaking tests if in future we tried to customize
the keys. Instead of passing keys we pass their corresponding commands
that is more flexible and resistant to breakage on changing keys.

Fixes zulip#469.
zee-bit added a commit to zee-bit/zulip-terminal that referenced this issue Jun 25, 2021
@zee-bit
refactor: ui/views/boxes: Replace hardcoded key in keypress to commands.
2e6f8b3 
This commit replaces all instances of hard-coded keys passed to keypress
that posed a risk of breaking tests if in future we tried to customize
the keys. Instead of passing keys we pass their corresponding commands
that is more flexible and resistant to breakage on changing keys.

Tests amended.

Fixes zulip#469.
zee-bit added a commit to zee-bit/zulip-terminal that referenced this issue Jul 4, 2021
@zee-bit
refactor: views: Migrate mouse_events to use primary_key_for_command.
982207c 
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
@zee-bit
refactor: ui/views/boxes: Replace hardcoded key in keypress to commands.
80b0889 
This commit replaces all instances of hard-coded keys passed to keypress
that posed a risk of breaking tests if in future we tried to customize
the keys. Instead of passing keys we pass their corresponding commands
that is more flexible and resistant to breakage on changing keys.

Tests amended.

Fixes zulip#469.
zee-bit added a commit to zee-bit/zulip-terminal that referenced this issue Jul 7, 2021
@zee-bit
refactor: views: Migrate mouse_events to use primary_key_for_command.
e6dc355 
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
@zee-bit
refactor: ui/views/boxes: Replace hardcoded key in keypress to commands.
6600cef 
This commit replaces all instances of hard-coded keys passed to keypress
that posed a risk of breaking tests if in future we tried to customize
the keys. Instead of passing keys we pass their corresponding commands
that is more flexible and resistant to breakage on changing keys.

Tests amended.

Fixes zulip#469.
zee-bit added a commit to zee-bit/zulip-terminal that referenced this issue Jul 8, 2021
@zee-bit
refactor: views: Migrate mouse_events to use primary_key_for_command.
fcadc80 
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
@zee-bit
refactor: ui/views/boxes: Replace hardcoded key in keypress to commands.
ffc5aaa 
This commit replaces all instances of hard-coded keys passed to keypress
that posed a risk of breaking tests if in future we tried to customize
the keys. Instead of passing keys we pass their corresponding commands
that is more flexible and resistant to breakage on changing keys.

Tests amended.

Fixes zulip#469.
@neiljp neiljp added this to the Next Release milestone Jul 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area: tests
Projects
None yet
Milestone
0.7.0
4 participants
@neiljp @zulipbot @preetmishra @zee-bit