Open johnnychen94 opened 4 years ago
In the former case, I think we should verify to match one "specific" reference rather than "any" one. What about adding the compatible package versions as a suffix as follows?
In practice, I think we only need to support a limited set of rules. Complex rules are not suitable for handling within filenames. The users can still write complex logic in their test code.
Thanks for the enlightening feedback, and the one in #66. Unfortunately, I've become exhausted to continue the enhancement of this package recently because of a series of day-to-day seminars. If things go smoothly, I'll come back and continue these in September.
In practice, I think we only need to support a limited set of rules. Complex rules are not suitable for handling within filenames. The users can still write complex logic in their test code.
This issue right now is at an awkward status that if we support this, then the regeneration workflow #52 with rm -rf test/references
won't work perfectly; if there are multiple references candidates, then we could only generate one in each setting, while the others can only be restored using git (this works but is not perfect as those removed references can be outdated).
I now prefer to use more eager pre-processing rules and the by
keyword more often to filter out and ignore irrelevant information. Just as I've done in https://github.com/JuliaTesting/ReferenceTests.jl/pull/66/files#diff-83b01706c3eadcbd57f11f30a83e1dacR3-R26 as you already noticed.
ReferenceTests.jl can determine if each existing reference file is valid or invalid (but not sure if that is what the user intended), so it can provide a smarter method of regeneration than rm -rf
workflow.
However, I don't think the concrete implementation needs to be discussed here for now.
I think providing pre-processors and their interfaces is a good idea. :+1: However, it is also essential to compare not "live" objects but human-readable "frozen" objects. As I wrote in the comment in the PR, I think that the references in reference tests should be rewritten extremely carefully and strictly.
However, I don't think the concrete implementation needs to be discussed here for now.
FWIW, the plan I have in mind is to first implement a match_reference
function that follows the same rule as @test_reference
but returns true
/false
as suggested in #47 and then see what can be improved after that.
Doing that could easily bring unexpected breaking changes, and that's why I want to do #66 first. :sigh:
This idea is growing on me. In https://github.com/oxinabox/LayeredLayouts.jl/pull/4 there are multiple equally good solutions and depending how the solver is feeling it can come up with different images
This can be useful in two cases:
actual = first(randperm(3))
)