Implications of returning more than a boolean following screenshot comparison?

[EranSch]

Hey there!

First off, this project is awesome!

I've been exploring routes for building a screenshot/diff utility to provide interactive feedback based on mismatched screenshots. This was the first project I could see using, based on flexibility and overall performance.

In my implementation, it would be really helpful if the result returned following a call to toMatchSnapshot contained a path to the diff image. I forked and tested the change for my use case but wanted to share it here before proposing a PR, as I'm not sure if it impedes other use cases.

Below is my branch, I'm happy to incorporate feedback if this seems like a reasonable change.

https://github.com/Swingline0/differencify/tree/result-data (Diff)

Instead of a boolean, my change returns the actual result object so it looks more like this:

"result": {
    "diffPath": "[...]/Homepage 1.differencified.png",
    "matched": false
}

Thanks again for this awesome and inspiring work!

[NimaSoroush]

Hi @Swingline0: Thanks for the suggestion and implementing the feature in your branch. Can you explain me your usecase in a bit more detail? Just trying to understand why a diffPath in result object would be more useful then getting the diff image from defult __differencified_output__ path.

I am happy to get that feature into Differencify but without introducing a breaking change. Maybe it makes more sense to return metadata as a callback method whithin toMatchSnapshot.

await differencify
    .init(TestOptions)
    .launch()
    .newPage()
    .setViewport({ width: 1600, height: 1200 })
    .goto('https://github.com/NimaSoroush/differencify')
    .waitFor(1000)
    .screenshot()
    .toMatchSnapshot((result) => {
      console.log(result); // Prints test result data, either:
      // { matched: true }
      // { matched: false, diffPath: 'path/to/diff' }
      // { updated: true }
    })
    .close()
    .end();

Or having a separate method just for this purpose

await differencify
    .init(TestOptions)
    .launch()
    .newPage()
    .setViewport({ width: 1600, height: 1200 })
    .goto('https://github.com/NimaSoroush/differencify')
    .waitFor(1000)
    .screenshot()
    .toMatchSnapshot()
    .matchResult((result) => {
      console.log(result); // Prints test result data, either:
      // { matched: true }
      // { matched: false, diffPath: 'path/to/diff' }
      // { updated: true }
    })
    .close()
    .end();

Let me know your thoughts

[EranSch]

Thanks, @NimaSoroush!

My use case deviates from the traditional approach as it seeks to implement differencify more as a service rather than a CI/local utility. So, say, post-deployment the service can be notified of an update and dispatch notifications in the event of a change.

As such, I have been looking for a manner to retrieve a full path to the diff image, programmatically, in order to further process it and send it along on its way.

Of the two example implementations you proposed, I suppose I would push for the former of the two, supporting a callback with the toMatchSnapshot method... purely just to avoid adding addition methods that could lead to some confusion.

If you agree on this approach, I'm happy to revise my branch for a proper implementation for your review.

Thanks!

[NimaSoroush]

Thanks for your explanation @Swingline0. I will more than happy to see the PR and also providing the example in API.md and Integration.test.js files. I am pretty sure it will be useful for the other people as well. If you would like to run integration tests, it is docker based and it been explained in CONTRIBUTION.md

[EranSch]

Closing as #83 is now merged