Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

@test_reference should fail when reference file is missing #127

Open
omus opened this issue Apr 26, 2024 · 7 comments
Open

@test_reference should fail when reference file is missing #127

omus opened this issue Apr 26, 2024 · 7 comments
Labels
breaking Changing this would be a breaking feature enhancement

Comments

@omus
Copy link
Member

omus commented Apr 26, 2024

Calling @test_reference on a non-existent file when JULIA_REFERENCETESTS_UPDATE is unset should cause a test failure instead of generating a new reference file:

julia> @test_reference "dne.txt" "hello"
┌ Info: Reference file for "dne.txt" did not exist. It has been created:- NEW CONTENT -----------------
│ hello
│ -------------------------------
└   new_reference = "/Users/cvogt/.julia/dev/TestReports/dne.txt"
[ Info: Please run the tests again for any changes to take effect

The rational behind this change is that if a user forgets to commit a reference file CI jobs will pass even though the reference test is broken. This happened to me in: JuliaTesting/TestReports.jl#99

@oxinabox
Copy link
Member

oxinabox commented Apr 28, 2024

This is intentional, but perhaps not the wisest feature.
It was there to make it easy to initially create the test files,
or after a major change recreate all the ones you knew should have changed (by deleting old ones manually then running tests.)
With the logic that you could just check the changes in git.

It was added long before #110
Now that we have that, I can see the argument for instead requiring that the user set ENV["JULIA_REFERENCETESTS_UPDATE"]=true
but I would say it would be a breaking change, since i know I at least depend on this feature as part of my standard workflow.
(but I could learn to change it)

@oxinabox
Copy link
Member

as an alternative, possibly we should refuse to create new reference files if ENV["CI"] is true?

@oxinabox oxinabox added the breaking Changing this would be a breaking feature label Apr 28, 2024
@omus
Copy link
Member Author

omus commented Apr 29, 2024

as an alternative, possibly we should refuse to create new reference files if ENV["CI"] is true?

I think this is a reasonable compromise but would lead to some user confusion in the long term.

@oxinabox
Copy link
Member

I think we can just drop automatically creating if missing and the JULIA_REFERENCETESTS_UPDATE enviroment var not set
and make this a breaking change -- probably go straight to v1.0.0 with this, since this is the only breaking change we have wanted in years i think we have hit that level of stability.

What do you think @johnnychen94 ?

@johnnychen94
Copy link
Member

@oxinabox I'm all good with what you think is good. (Actually, I don't have the bandwidth to proceed with open source projects now 😭)

@oxinabox
Copy link
Member

Cool well I am happy to go with doing this.

@oxinabox
Copy link
Member

For my own notes this is functionally reverting
#52

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Changing this would be a breaking feature enhancement
Projects
None yet
Development

No branches or pull requests

3 participants