happo.io icon indicating copy to clipboard operation
happo.io copied to clipboard

[Feature request] Run Function After Comparison Request

Open wokayme opened this issue 3 years ago • 6 comments

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

wokayme avatar Sep 14 '22 06:09 wokayme

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?

trotzig avatar Sep 14 '22 07:09 trotzig

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.

wokayme avatar Sep 14 '22 07:09 wokayme

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?

trotzig avatar Sep 14 '22 07:09 trotzig

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 avatar Sep 14 '22 08:09 wokayme

@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 ;)

wokayme avatar Sep 22 '22 07:09 wokayme

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 [email protected]. I was probably going to mimic what github does:

  • have one endpoint for all webhooks (a type property decides what type of event is being sent)
  • allow multiple subscribers (URLs) per account
  • store calls so that users can debug and retry individual calls through a UI
  • sign the payload so that you can be sure the call is coming from the right origin

Let me know if you have other ideas!

trotzig avatar Sep 22 '22 12:09 trotzig