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

[WIP] test_reference_broken #38

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

johnnychen94
Copy link
Member

@johnnychen94 johnnychen94 commented Oct 12, 2019

Codes here are not ready for review (most of the part is copy&paste).

@test_reference_broken is introduced here, and it works like @test_broken.


Some details needs to be clarified and added. But I'd like to show it as a demo of an alternative to #12

A test result:

(ReferenceTests) pkg> test
  Updating registry at `~/.julia/registries/General`
  Updating git-repo `https://github.com/JuliaRegistries/General.git`
   Testing ReferenceTests
 Resolving package versions...
[ Info: Nine tests should correctly report failure in the transcript (but not the test summary).
Skipping Base.active_repl
Skipping Base.active_repl_backend
Skipping Base.active_repl
Skipping Base.active_repl_backend
┌ Warning: test fails because PSNR -15.415644645690918 < 25
└ @ ReferenceTests ~/Documents/Julia/ReferenceTests.jl/src/equality_metrics.jl:45
┌ Warning: test fails because size(ref) (64, 64) != size(x) (512, 512)
└ @ ReferenceTests ~/Documents/Julia/ReferenceTests.jl/src/equality_metrics.jl:38
Test Summary:  | Pass  Broken  Total
ReferenceTests |   31       9     40
   Testing ReferenceTests tests passed 

@coveralls
Copy link

Coverage Status

Coverage decreased (-15.9%) to 66.385% when pulling efdd67c on johnnychen94:broken into 46d650a on Evizero:master.

@oxinabox
Copy link
Member

I wonder if we shoukld infact switch to having a function that returns true or false,
or throws an error, depending.

Then we could do:

@test matches_reference(actual, "reference.txt")
@test_broken matches_reference(actual, "reference.txt")
@test_skip matches_reference(actual, "reference.txt")
@test !matches_reference(actual, "reference.txt")

rather than having to recrate each.

Are we getting value out of having a top-level macro that we couldn't get out of a function?

@johnnychen94
Copy link
Member Author

I wonder if we shoukld infact switch to having a function that returns true or false,
or throws an error, depending.

I like this idea a lot; I've been continuing thinking of replacing @test_reference by test_reference these days.

So export test_reference and deprecate @test_reference?

@oxinabox
Copy link
Member

yeah, i think so.
We might want to bike-shed on the name a bit first though.

@test test_reference(actual, "reference.txt")

to me doesn't feel right.

and

@test !test_reference(actual, "reference.txt")

feels really wrong. (It is a weird thing to do in the first place but anyway...)

Definately will want a deprecation this time.
Which means first tagging a nonbreaking relese with the deprecation
then a breaking release (at some point) with the deprecation removed.

This was referenced Aug 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants