happo / happo.io

Happo is a cross-browser screenshot testing service
https://happo.io
MIT License
196 stars 25 forks source link

[Feature request] Run Function After Comparison Request #241

Open wokayme opened 2 years ago

wokayme commented 2 years ago

I would like to be able to pass to happo config function which will be triggered after sending a synchronous request to the Happo repository.

Usecase

handle statistics about flaky/failing tests for internal needs.

Currently how I can do it

Currently, the thing I am getting back is a simple summary where I need on my own look for data and parse it. IMO it's quite dangerous as I don't know how API is going to change and this simple text can be changed.

Proposed solution

Add the opportunity to pass a function in .happo.js config file which will be run after comparison and get as an argument https://happo.io/docs/api#Comparison response object.

Example PR for better illustration. I will be happy with the writing implementation, but before I would like to hear feedback:

https://github.com/happo/happo.io/pull/242

trotzig commented 2 years ago

Hi @wokayme, and thanks for the feature request!

This would definitely make sense. My main concern is that we've made async comparisons the default, which means the comparison result will not be available unless you set HAPPO_IS_ASYNC=false.

A similar idea that's been floating around is to add a way to get a callback on a URL when a result is available. This is similar to GitHub's webhooks. Would that solve your usecase you think?

wokayme commented 2 years ago

In our case, we have HAPPO_IS_ASYNC set for false so it's not a problem for us.

But I think creating a webhook is quite a good idea:

  1. We have unification for both parameters
  2. People can try to write their own CI logic if it's more complicated

The only problem is that it requires extending API https://happo.io/docs/api.

trotzig commented 2 years ago

Yeah, it's slightly more involved. I'm happy adding a callback in the meantime. How about we call it afterSyncComparison to make it a little more obvious that it's only for the sync case?

wokayme commented 2 years ago

I added changes ;) happy to hear feedback and potential comments to units. I didn't know how to write them keeping form 🤔 If we want to change them I would be happy to hear suggestion

wokayme commented 2 years ago

@trotzig is any timeline for webhooks for job comparison 🤔 We are currently using afterSyncComparison what works well for places where we use happo-cli, unfortunately we want to do similar functionality for e2e cases. Currently I see that we are using there async-report endpoint which is not documented in API documentation. I can of course write my own way of collecting snapshots from e2e and later send it with synchronic compare status, but I wouldn't like to do it and I would prefer to use an official solution for it than a hacking e2e package ;)

trotzig commented 2 years ago

Yeah it will be hard to get happo-e2e to honor the new afterSyncComparison callback. Let me work out a plan internally for the webhooks, I think we should be able to start working on that soon. If you have any input, let me know here or send an email to henric@happo.io. I was probably going to mimic what github does:

Let me know if you have other ideas!