r-lib / vdiffr

Visual regression testing and graphical diffing with testthat
https://vdiffr.r-lib.org
Other
185 stars 31 forks source link

A new visual case should require review before it is accepted #114

Open wilkox opened 3 years ago

wilkox commented 3 years ago

Before vdiffr began using testthat's file snapshot mechanism, when a new expect_doppelganger() expectation was added it would have to be inspected and accepted with manage_cases() before the test would pass. Now, the snapshot created the first time the test is run is saved as the reference, and so implicitly assumed to be correct. This makes it difficult to add new failing cases. It also creates a danger that bugs will be missed if test() happens to be run for the first time before the bug is noticed.

Ideally, new cases should require confirmation in the snapshot_review() app before they pass. Alternatively, it would be useful to have a function to mark new cases as failing. Unfortunately it seems like testthat does not have any way to mark a file snapshot as 'bad', but only recognises files as 'new' (compared to the reference file). Perhaps the first time expect_doppleganger() is run it could create a blank SVG as the reference file?

lionel- commented 3 years ago

Perhaps file a testthat issue since this is about the snapshotting workflow in general rather than vdiffr? Snapshotting errors during development has the same behaviour. Note that you can use git to review and discard tentative snapshots.