mobile-dev-inc / maestro

Painless Mobile UI Automation
https://maestro.mobile.dev/
Apache License 2.0
5.89k stars 281 forks source link

feat: add `cropOn` screenshots capability #2086

Open TheKohan opened 1 month ago

TheKohan commented 1 month ago

Proposed changes

This PR extends current capabilities of takeScreenshot command with cropOn prop which is of an ElementSelector type.

Motivation

Visual regression testing is something that has been on the horizon for some time #1222. This PR is aiming to fill the gaps and prepare the ground for further development in this area (will fit #2078 great). Component level screenshots are something that will greatly decrease screenshot size and make component-level tests possible.

Example usage

test-screenshot.yml

appId: com.example.app
---
- launchApp
- tapOn: Playground
- takeScreenshot:
    path: 'Playground'
 ║                                                             
 ║  > Flow: test-flow                                          
 ║                                                             
 ║    ✅   Launch app "com.exapmle.com"                                                        
 ║    ✅   Tap on "Playground"                         
 ║    ✅   Take screenshot Playground                          
 ║            

test-component-screenshot.yml

appId: com.example.app
---
- launchApp
- tapOn: Playground
- takeScreenshot: 
      path: "Markdown"
      cropOn:
        id: MarkdownInput_Example
 ║                                                             
 ║  > Flow: test-flow                                          
 ║                                                             
 ║    ✅   Launch app "com.exapmle.com"      
 ║    ✅   Tap on "Playground"
 ║    ✅   Take screenshot Markdown, cropped id: MarkdownInput_Example
Playground Markdown
image image

Testing

Matheeusb commented 1 month ago

Hey @TheKohan -- About the tests, you can add a E2E test in the "e2e" module. About this feature, great delivery!

Fishbowler commented 1 month ago

Is there a reason to limit it to id than reuse the existing complex element selector logic that exists for other commands?

TheKohan commented 1 month ago

Is there a reason to limit it to id than reuse the existing complex element selector logic that exists for other commands?

To be completely honest i didn't think about it, but i think you're right. Let me try to reimplement this part.

TheKohan commented 1 month ago

@Fishbowler - adjusted API acording to your suggestions, it's indeed way better this way. Thanks for the hint !

TheKohan commented 1 month ago

Added test cases.

TheKohan commented 1 month ago

@amanjeetsingh150 - can you review this PR ?

amanjeetsingh150 commented 2 weeks ago

Hey @TheKohan this LGTM, we will be trying to get this in 1.40.0 CLI version.

amanjeetsingh150 commented 2 weeks ago

Can you also raise a PR to update docs here:

https://github.com/mobile-dev-inc/maestro-docs

TheKohan commented 5 days ago

Thanks for answering, i'll try to find some time next week to cover things you've mentioned !