Is your feature request related to a problem?
I've looked into using this library for a while, but one blocker is that the name #screenshot conflicts with an existing helper method we've used extensively in our system specs for over ten years. In our system, #screenshot means "capture a screenshot with this name and description (and other attributes/options/metadata), and it will be inserted into an HTML page (or PDF) that describes the feature".
While we may want to eventually migrate all of our existing screenshots to use this library, that's much easier to justify when we can do it gradually rather than as a single big change.
Additionally, the #screenshot method name doesn't say anything about making an assertion, and it feels strange for the primary API to "hide" its implicit assertion. IMO, the primary API should either be assert_matches_screenshot name or assert_no_screenshot_changes for minitest or test-unit and expect(page).to match_screenshot(name) for rspec.
The documentation for assert_matches_screenshot calls it an "Alias for backward compatibility with older test suites", but I think that's not how it should be treated. IMO, where your tests are making an assertion, they should (usually) explicitly claim to be making an assertion.
This does not mean that I'd remove the implicit assertion. A test might be written to assert other more fundamental aspects of the functionality, and comparing screenshots is more of a safeguard than a strict assertion. (With rspec, sometimes I'll use example metadata as a "tag" to add this sort of implicit assertion automatically.)
Describe the solution you'd like
I'd prefer that this library stop using DSL#screenshot internally, so a user of the library could use their own definition for the #screenshot method without breaking this library. For compatibility, DSL#screenshot would still be defined. Currently assert_matches_screenshot is an alias for screenshot, but I would reverse that relationship.
Less important related changes
I'd also add capture_screenshot which never compares to the existing screenshot (looking at the code, this could probably be done by deleting the existing file prior to taking the screenshot?).
And I'd maybe also add a compare boolean option to #screenshot, so comparisons could be disabled. Whether or not #screenshot implicitly compares might be something that could be enabled or disabled in global config, or per test (or rspec example).
Because assert_matches_screenshot is explicitly making an assertion, it would not have an option to fully disable comparison. But, when the baseline comparison image is missing, assert_matches_screenshot should mark that test as pending (in the teardown).
Describe alternatives you've considered
I could rename screenshot everywhere in my existing codebase, prior to adding this library.
How would this benefit visual regression testing?
It would allow us to gradually migrate our existing screenshots into compatibility with this library, rather than needing it all to work as one big change.
Is your feature request related to a problem?
I've looked into using this library for a while, but one blocker is that the name
#screenshotconflicts with an existing helper method we've used extensively in our system specs for over ten years. In our system,#screenshotmeans "capture a screenshot with this name and description (and other attributes/options/metadata), and it will be inserted into an HTML page (or PDF) that describes the feature".While we may want to eventually migrate all of our existing screenshots to use this library, that's much easier to justify when we can do it gradually rather than as a single big change.
Additionally, the
#screenshotmethod name doesn't say anything about making an assertion, and it feels strange for the primary API to "hide" its implicit assertion. IMO, the primary API should either beassert_matches_screenshot nameorassert_no_screenshot_changesfor minitest or test-unit andexpect(page).to match_screenshot(name)for rspec.The documentation for
assert_matches_screenshotcalls it an "Alias for backward compatibility with older test suites", but I think that's not how it should be treated. IMO, where your tests are making an assertion, they should (usually) explicitly claim to be making an assertion.This does not mean that I'd remove the implicit assertion. A test might be written to assert other more fundamental aspects of the functionality, and comparing screenshots is more of a safeguard than a strict assertion. (With rspec, sometimes I'll use example metadata as a "tag" to add this sort of implicit assertion automatically.)
Describe the solution you'd like
I'd prefer that this library stop using
DSL#screenshotinternally, so a user of the library could use their own definition for the#screenshotmethod without breaking this library. For compatibility,DSL#screenshotwould still be defined. Currentlyassert_matches_screenshotis an alias forscreenshot, but I would reverse that relationship.Less important related changes
I'd also add
capture_screenshotwhich never compares to the existing screenshot (looking at the code, this could probably be done by deleting the existing file prior to taking the screenshot?).And I'd maybe also add a
compareboolean option to#screenshot, so comparisons could be disabled. Whether or not#screenshotimplicitly compares might be something that could be enabled or disabled in global config, or per test (or rspec example).Because
assert_matches_screenshotis explicitly making an assertion, it would not have an option to fully disable comparison. But, when the baseline comparison image is missing,assert_matches_screenshotshould mark that test as pending (in the teardown).Describe alternatives you've considered
I could rename
screenshoteverywhere in my existing codebase, prior to adding this library.How would this benefit visual regression testing?
It would allow us to gradually migrate our existing screenshots into compatibility with this library, rather than needing it all to work as one big change.