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

Extend the navigate_until_text method #151

Open
aido opened this issue Oct 11, 2023 · 6 comments
Open

Extend the navigate_until_text method #151

aido opened this issue Oct 11, 2023 · 6 comments

Comments

@aido
Copy link

aido commented Oct 11, 2023

Would it be possible to extend the navigate_until_text method to navigate_until_text_at_position where x and y positions are provided?
Or use the same navigate_until_text method but add x and y parameters. Ignore position if x and y are not provided but test if text is at that position if x and y are provided.

@xchapron-ledger
Copy link
Contributor

Hello,
I guess you are trying to distinguished two screen with partially the same content?
Can you share the screen snapshots here?

Do you know that navigate_until_text now support searching for a regex? This might help distinguish screens.
Also, there is the navigate_until_snap function, though I don't really recommend using it unless really necessary.

Regarding your proposal, we can discuss about change in navigate_until_text, but we have to keep in mind that ragger tests needs to be as functional as possible on tests run on device through physical_backend where it is hard for an user to check for x and y values.

@aido
Copy link
Author

aido commented Oct 12, 2023

Hi @xchapron-ledger,

I guess you are trying to distinguished two screen with partially the same content?

Take for example the app-recovery-check application. A user scrolls left-to-right through an alphabet before choosing a letter. When the letter is at a certain position on screen the user chooses it.
Rather than use the compare_screen_with_snapshot or navigate_until_snapmethods with lots of snapshots a test could use "right button press until letter/text is at position x"

@xchapron-ledger
Copy link
Contributor

Take for example the app-recovery-check application. A user scrolls left-to-right through an alphabet before choosing a letter. When the letter is at a certain position on screen the user chooses it.
Rather than use the compare_screen_with_snapshot or navigate_until_snapmethods with lots of snapshots a test could use "right button press until letter/text is at position x"

Note that current ragger navigation is using a screen comparison between each navigation instruction to make sure the previous instruction has been processed and the device is ready to receive the next.

So I think in your situation I would have go with a compare_screen_with_snapshot, with a reference snapshot for each letter of the alphabet.

@lpascal-ledger You worked on the password app and the recovery one, do you have any opinion on this?
What do you think about precise coordinate text searching on the screen? I think it won't looks great on physical backend, but hopefully this stays a niche usage, and therefore it doesn't really matter?

@lpascal-ledger
Copy link
Contributor

lpascal-ledger commented Oct 13, 2023

This discussion pulls a lot of interesting questions on the Navigator.navigate* methods.

Personally I use Navigator.navigate* methods primarily to check the whole interface, so I didn't had needs for text position when I wrote the tests.

On a pure functional level, it would make sense to me to allow checking the text position. This information is provided by Speculos through its events, and so backends could allow some kind of event filtering on methods like compare_screen_with_text (eventually cascading to this function in the case of SpeculosBackend).
Implementing it for physical backend would be a bit complicated, but possible - and it is nice but niche alright.

Abstraction-wise, this feel too specific and low-level for the Navigator methods. I'm already not very comfortable with the current Navigator.navigate* granularity which I find complex enough as it is. I fear adding additional specific behavior would add confusing complexity and call for over specific behaviors.
A possible path to bring flexibility without adding a new method every time could be to have a Navigate.navigate_until(calback) method. User would give a callback() -> bool that would be called at every step to check if the stop condition is reached. That would allow to check for snapshot, text, text at specific position (given the backends offer such capability) or any other specific need.
What do you think of this?

If the expected result is to fasten the test execution however, as @xchapron-ledger mentionned currently they won't be performance benefits from waiting for this specifically positioned text. Snapshot will still be asked to Speculos, with the induced delay we know on graphic tests.

@aido
Copy link
Author

aido commented Oct 13, 2023

HI @lpascal-ledger,

Just to add that this suggestion may be niche but not so niche that it would be useful in only one app; app-recovery-check.
I have written an app called app-seed-tool that is based on app-recovery-check where a navigate_until_text_at_position method would also be useful.

@lpascal-ledger
Copy link
Contributor

@aido the niche thing I'm talking about is implementing BackendInterface.compare_screen_with_text with a specific position on physical backends, not the suggestion itself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants