Separate snapshot creation from comparison.

[mildfuzz]

Trying to add this into a pipeline process. I would like to add a step that creates and commits new snapshots as a discrete step to comparing snapshots. Is this possible as is or is this a feature request?

[moshensky]

Hi @mildfuzz,

I'm afraid that I might have not understood your issue. From the docs:

When function is executed it has following side effects:

• In absence of a previous snapshot file it converts pdf to an image, saves it as a snapshot and returns true
• If there is a snapshot, then pdf is converted to an image and gets compared to the snapshot:
  • if they differ function returns false and creates next to the snapshot image two other versions with suffixes new and diff. new one is the current view of the pdf as an image, where diff shows the difference between the snapshot and new images
  • if they are equal function returns true and in case there are new and diff versions persisted it deletes them

Could you elaborate a bit more please?

[mildfuzz]

Hi @mildfuzz,

I'm afraid that I might have not understood your issue. From the docs:

When function is executed it has following side effects:

• In absence of a previous snapshot file it converts pdf to an image, saves it as a snapshot and returns true
• If there is a snapshot, then pdf is converted to an image and gets compared to the snapshot:
  • if they differ function returns false and creates next to the snapshot image two other versions with suffixes new and diff. new one is the current view of the pdf as an image, where diff shows the difference between the snapshot and new images
  • if they are equal function returns true and in case there are new and diff versions persisted it deletes them

Could you elaborate a bit more please?

I need the actual test (as in, comparison to existing image) to be a discrete step from snapshot creation

In fact, ideally, I would like the tests to fail in the absence of an existing snapshot

This is so I can make the two separate stages in a pipeline

[moshensky]

Just to confirm: an option to run and fail a test in the absence of an existing snapshot instead of creating one will be sufficient, right?

[mildfuzz]

Don't think so, would also need an option to do the reverse and only create new ones. Don't run tests.

[moshensky]

Let's see if I understand this correctly after rereading your first post. 🤦

Some context:

1. There are existing tests with known snapshots.
2. A new test is added without a snapshot.
3. Meanwhile, a change to the code causes one of the tests from the first point to fail.

You want the ability to run tests so that all those with existing snapshots are skipped, meaning the test from the third point won't fail. At the same time, any new tests that were added, such as the one from the second point, should generate a new snapshot.

Then you want to run all the tests again, and they should behave as they do now.

Are we getting closer? 😅

[mildfuzz]

Yeah, we're close. I think the language is difficult here, as you refer to producing new snapshots as tests, but that's not how I see it. They're more an initialisation step.

1) A step that ONLY produces new snapshots where required (for new tests). Does not fail snapshots that have changed (doesn't actually need to do any comparison, but this is an implementation detail) 2) A step that ONLY runs existing snapshots. Does not produce new one

As a stretch, it might be handy if the second step failed when it found missing snapshots, but this is not a requirement

[moshensky]

A new optional prop snapshotMode could be added to the comparePdfToSnapshot compareImageOpts argument:

snapshotMode?: 
    | 'compare-or-generate-reference' // Default behaviour: compares existing snapshots or generates new references if missing.
    | 'create-new-references-only' // Only creates new snapshots where ones are missing. Always returns `true`.
    | 'compare-only' // Only compares existing snapshots. Returns `true` if a reference is missing but doesn't create a new one.
    | 'compare-and-fail-on-missing' // Only compares existing snapshots. Returns `false` if a reference is missing, without creating a new one.

[moshensky]

A new optional prop snapshotMode could be added to the comparePdfToSnapshot compareImageOpts argument:

snapshotMode?: 
    | 'compare-or-generate-reference' // Default behaviour: compares existing snapshots or generates new references if missing.
    | 'create-new-references-only' // Only creates new snapshots where ones are missing. Always returns `true`.
    | 'compare-only' // Only compares existing snapshots. Returns `true` if a reference is missing but doesn't create a new one.
    | 'compare-and-fail-on-missing' // Only compares existing snapshots. Returns `false` if a reference is missing, without creating a new one.

I'm not sure how much value this will add. It shouldn't be that big of a cost to support, but it will add some complexity. I guess then it will be up to you to use for example some env variables to set that prop and achieve the behaviour that you desire.

Nevertheless I wonder, wouldn't you want during development, when adding new test to run them and review that the reference image generated is the one that you want and commit it together with the new test?

[mildfuzz]

Sorry for not coming back to this sooner.

It's about being able to use this to catch changes during a release cycle. In a pipeline, Jenkins (or whatever you're using) would create a snapshot, presume its truthiness and the stage will pass. The new snapshot would not be committed unless we added that to the stage and therefore, this stage would continue to pass until someone runs it locally and commits.

What this means is that if I create a new PDF template without a snapshot, the stage passes and I can release. If someone makes an unexpected change, the stage will still pass and they can still release.

I actually think the creation of snapshots should not be the default, for the above reason, as it presumes a pass when no snapshots exist, but I be fine to just be able to turn this off in specific contexts.

I remember one of my first seniors saying something to me when I was first starting which really stuck with me: "fail quickly, fail violently"

[mildfuzz]

Something like this? https://github.com/moshensky/pdf-visual-diff/pull/60

[mildfuzz]

I appreciate this has differed slightly from the original title, but I think this ACTUALLY is closer to what I need to make this safe. Edited title.

[moshensky]

Hi @mildfuzz, I will have a look later today or tomorrow.

[moshensky]

@mildfuzz I've left a couple of comments in the PR.