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

[Feature request] Run Function After Comparison Request #241

Open
wokayme opened this issue Sep 14, 2022 · 6 comments
Open

[Feature request] Run Function After Comparison Request #241

wokayme opened this issue Sep 14, 2022 · 6 comments

Comments

@wokayme
Copy link

wokayme commented Sep 14, 2022

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:

#242

@trotzig
Copy link
Contributor

trotzig commented Sep 14, 2022

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
Copy link
Author

wokayme commented Sep 14, 2022

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
Copy link
Contributor

trotzig commented Sep 14, 2022

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
Copy link
Author

wokayme commented Sep 14, 2022

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
Copy link
Author

wokayme commented Sep 22, 2022

@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
Copy link
Contributor

trotzig commented Sep 22, 2022

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!

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

2 participants