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

Modularize tests buttons module #4740

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

bieleluk
Copy link
Contributor

@bieleluk bieleluk commented Mar 5, 2025

This PR prepares buttons test module for Eckhart layout support that has a different screen resolution and different buttons positions. This is done by creating functions with layout_type param instead of using constants.
On top of that, it refactors set_selection function to accommodate to this change.

Copy link

github-actions bot commented Mar 5, 2025

core UI changes device test click test persistence test
T2T1 Model T test(screens) main(screens) test(screens) main(screens) test(screens) main(screens)
T3B1 Safe 3 test(screens) main(screens) test(screens) main(screens) test(screens) main(screens)
T3T1 Safe 5 test(screens) main(screens) test(screens) main(screens) test(screens) main(screens)
All main(screens)

@bieleluk bieleluk force-pushed the bieleluk/test-modularization branch 6 times, most recently from 4d1eb6c to 0ebb661 Compare March 6, 2025 10:13
@bieleluk bieleluk force-pushed the bieleluk/test-modularization branch from 0ebb661 to ebe1c59 Compare March 6, 2025 11:41
@bieleluk bieleluk self-assigned this Mar 6, 2025
@bieleluk bieleluk added the tests Automated integration tests label Mar 6, 2025
@bieleluk bieleluk requested review from romanz and obrusvit March 6, 2025 11:49
@bieleluk bieleluk changed the title WIP: chore(tests): modularize tests buttons module Modularize tests buttons module Mar 6, 2025
@bieleluk bieleluk marked this pull request as ready for review March 6, 2025 12:22
@bieleluk bieleluk requested a review from matejcik as a code owner March 6, 2025 12:22
Copy link
Contributor

@romanz romanz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for the refactoring!
Added one suggestion and a few nits.

def grid34(x: int, y: int) -> Coords:
return grid(DISPLAY_WIDTH, 3, x), grid(DISPLAY_HEIGHT, 4, y)
def grid34(x: int, y: int, layout_type: LayoutType) -> Coords:
assert layout_type in (LayoutType.Bolt, LayoutType.Delizia)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: can be removed, since it is also checked in display_width() & display_height().
(here and below)



# Horizontal coordinates
def left(layout_type: LayoutType):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: add return type annotation?
(here and below)

def grid(dim: int, grid_cells: int, cell: int) -> int:
step = dim // grid_cells
ofs = step // 2
return cell * step + ofs


def grid35(x: int, y: int) -> Coords:
return grid(DISPLAY_WIDTH, 3, x), grid(DISPLAY_HEIGHT, 5, y)
def grid35(x: int, y: int, layout_type: LayoutType) -> Coords:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it would simpler to add a "Buttons" class, that will have all the functions here as its methods, e.g.:

class Buttons:
    def __init__(self, layout_type: LayoutType):
        self.width = display_height(layout_type)
        self.height = display_height(layout_type)
        
    def grid35(self, x: int, y: int) -> Coords:
        return grid(self.width, 3, x), grid(self.height, 5, y)
    
    ...
    
    def info(self) -> Coords:
        return (self.mid(), self.bottom())
        
    ...

So that you could do something like:

   buttons = Buttons(debug.layout_type)  # can be done once
   debug.click(buttons.cancel())  # no need to pass layout type :)

WDYT?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests Automated integration tests
Projects
Status: 🏃‍♀️ In progress
Development

Successfully merging this pull request may close these issues.

2 participants