search

approvals / ApprovalTests.cpp

Native ApprovalTests for C++ on Linux, Mac and Windows
https://approvaltestscpp.readthedocs.io/en/latest/
Apache License 2.0
316 stars 51 forks source link

File names could be improved to detect or prevent unintended duplicate names #97

Closed p-podsiadly closed 3 years ago

p-podsiadly commented 4 years ago

Background: I have two test cases: scale > 1 and scale < 1. Files generated for both test cases have the same file name: "scale___1.received.png" (both < and > are replaced with underscore).

This could be improved. For example, instead of replacing each forbidden character with underscore, ApprovalTestNamer could use XML/HTML character references, so that files in the above example would be named scale_&gt;_1.received.png and scale_&lt;_1.received.png.

claremacrae commented 4 years ago

Thanks, I don't think we thought about the use of _ resulting in similar tests giving non-unique file names.

I'd be super nervous about changing the naming convention, though, as it could break the results of existing tests for existing users...

Based on this answer, I'd hesitate to use & in a file name: https://superuser.com/a/748264

[]()^ #%&!@:+={}'~ and [`] all have special meanings in many shells, and are annoying to work around, and so should be avoided.

claremacrae commented 4 years ago

I know it's not a totally general solution, but it's possible to a register custom namer: https://github.com/approvals/ApprovalTests.cpp/blob/master/doc/Namers.md#registering-a-custom-namer

claremacrae commented 4 years ago

For clarity, I do agree we should look at this - I’m just not sure what approach to take.

claremacrae commented 4 years ago

An alternative idea would be for us to keep track of all the approval filenames used in a session, and print out a warning if a duplicate filename is encountered.

This would also help users who have two Approvals::verify() calls in the same scope...

It doesn't solve the problem in this ticket, but it does help users track down the cause of the error.

We could even record both the original test name, and the generated file name - so if there are duplicated filenames, we could print out all the matching test names.

isidore commented 3 years ago

Thank you for pointing out this issue.

We have added a customization point that will allow users to handle this in the method of their choice. Here are the docs Converting Test Names to Valid FileNames