maestro icon indicating copy to clipboard operation
maestro copied to clipboard

feat: add new `assertVisual` command

Open TheKohan opened this issue 1 year ago • 7 comments

Proposed changes

This PR is built on top of #1898 by @bartekpacia and implements a basic assertVisual flow which is much awaited in the community (#1222).

Given a simple React Native app

app.tsx

export default function App() {
  const [toggle, setToggle] = React.useState(false);
  return (
    <View style={styles.container}>
      <View style={[styles.dot, {backgroundColor: toggle ? 'red' : 'blue'}]}>
        <Button title="test" onPress={() => setToggle(!toggle)}></Button>
      </View>
    </View>
  );
}

const styles = StyleSheet.create({
  container: {
    flex: 1,
    alignItems: 'center',
    marginTop: 200,
  },
  dot: {
    backgroundColor: 'red',
    padding: 30,
    borderRadius: '50%',
  },
});

and a flow

test-flow.yml

appId: com.example.app
---
- launchApp
- tapOn: 'test'
- assertVisual: Login screen # compares the current screen against `.maestro/visual_regression/Login screen.png`

maestro will compare screenshots, producing result image with differences masking.

 ║  > Flow: test-flow                                             
 ║                                                                
 ║    ✅   Launch app "com.example.app"                                                              
 ║    ✅   Tap on "test"                                          
 ║    ✅   Tap on "test"                                          
 ║    ❌   Assert visual difference with baseline Login screen (threshold: 95%)                                                     
 ║   
Baseline Current Result
Login screen image Login screen

Result for now is being saved in .maestro/failed_visual_regression/ directory.

If there's no baseline, current will be saved as one.

What is missing:

  • changes accepting flow
  • cloud integration
  • testing
  • remove system-ui elements (like status-bar)
  • component-scoped tests (partial screenshots for component of given test-id)

Testing

  • in progress - test plan has not yet been laid out due to potentially big changes in the proposed flow.

Issues fixed

  • #1222

TheKohan avatar Oct 07 '24 10:10 TheKohan

You've mentioned missing the "changes accepting flow". What have you got in mind for the workflow for a failure where the new "look" is as-intended?

Fishbowler avatar Oct 08 '24 10:10 Fishbowler

You've mentioned missing the "changes accepting flow". What have you got in mind for the workflow for a failure where the new "look" is as-intended?

@Fishbowler - the only thing i could've think of as of right now is to keep baseline in repository and in case of expected diff fail CI with a message and let the user manually delete current baseline. #1222 - here are some takes on this problem also.

TheKohan avatar Oct 08 '24 11:10 TheKohan

You've mentioned missing the "changes accepting flow". What have you got in mind for the workflow for a failure where the new "look" is as-intended?

@Fishbowler - the only thing i could've think of as of right now is to keep baseline in repository and in case of expected diff fail CI with a message and let the user manually delete current baseline. #1222 - here are some takes on this problem also.

We would use this asap in our project and for us modifying the "baseline" image as part of the PR would be our expected workflow, and would actually be a great way for a design/product stakeholder to review any of the baseline screenshot diffs as part of a PR. In other words, no additional build here at all is a perfectly good "accept changes" workflow for our team.

jtdaugh avatar Oct 08 '24 20:10 jtdaugh

@bartekpacia - would gladly accept review/guidance on this one. Also #2086 would be invaluable for this.

TheKohan avatar Oct 16 '24 10:10 TheKohan

Excellent to see some new features coming into Maestro, thank you @TheKohan 🙏

Could mix very well with this: #1526

rikur avatar Oct 18 '24 14:10 rikur

Hey @TheKohan – this is some exciting work. I no longer work at @mobile-dev-inc, and am a bit sad I didn't have time to finish this:(

cc @amanjeetsingh150 – he's the right person to review this PR:)

bartekpacia avatar Oct 18 '24 15:10 bartekpacia

@amanjeetsingh150 - can you review this PR ?

TheKohan avatar Oct 24 '24 13:10 TheKohan

Is there any update on this? Is this something planned for soon or not?

NikolaSimsic avatar Feb 12 '25 09:02 NikolaSimsic

This is a very very useful feature that I hope can be available soon 🙏

poponuts avatar Apr 11 '25 05:04 poponuts

Hey @TheKohan really appreciate you opening this PR. I'm picking up the review of this and will prioritize getting this merged. Some feedback before we can merge:

Let's update the API to the following so that it matches the takeScreenshot command in naming and API:

- assertScreenshot: screen.png
- assertScreenshot:
    path: screen.png
    cropOn: "Login"
    thresholdPercentage: 95.3

This would make the workflow for generating the baseline screenshots straightforward:

  • First run with takeScreenshot to generate the baseline images
  • Then change those commands to assertScreenshot and commit without needing to change anything else (optionally adding a threshold)

This also highlights another update that we should make in this PR:

  • This new command should also respect the Orchestra.screenshotsDir path just like the takeScreenshot command
  • If screenshotsDir isn't set it should default to current dir just like the takeScreenshot command
  • It should generate the diff image alongside the screenshot it's looking at (cwd or screenshotsDir)

Leland-Takamine avatar Nov 18 '25 22:11 Leland-Takamine