Add test_warn macro to Base.Test for checking for warnings #19903

stevengj · 2017-01-06T17:29:15Z

This adds a @test_warn macro to test for the presence of warnings, and @test_nowarn to test for the absence of warnings.

I updated the precompilation tests to use this macro, rather than the @async method they were previously using. I also used the macro to add a test for #19888 (similar to #18746, but much cleaner given the new macro).

kshyatt · 2017-01-06T17:34:28Z

base/test.jl

+"""
+    @test_warn msg expr
+
+Test whether evaluating `expr` results in `stderr` output that contains


stderr -> STDERR and link ?

Also, maybe include an example?

kshyatt · 2017-01-06T17:34:54Z

base/test.jl

+"""
+    @test_nowarn expr
+
+Test whether evaluating `expr` results in empty `stderr` output


Same here re stderr

kshyatt

LGTM! This is great!

…ne in test_nowarn

stevengj · 2017-01-06T19:15:50Z

Also added a commit to use @test_warn in the Pkg tests rather than a custom output-grabbing macro. The PR is now a net deletion of code.

tkelman · 2017-01-06T21:08:25Z

test/pkg.jl

        vers = Pkg.installed("Example")
+        branch = LibGit2.with(LibGit2.GitRepo, Pkg.dir("Example")) do repo
+          LibGit2.branch(repo)
+       end


strange indent

will fix, sorry

tkelman · 2017-01-06T21:09:05Z

test/pkg.jl

-        ret, out, err = @grab_outputs Pkg.pin("Example")
-        @test ret === nothing && out == ""
-        @test ismatch(r"INFO: Creating Example branch pinned\.[0-9a-f]{8}\.tmp", err)
-        @test !contains(err, "INFO: No packages to install, update or remove")


don't delete this test

It wasn't deleted, and in fact the test was strengthened: The new regex test has r"^...$" so that it checks that exactly one INFO message is printed.

tkelman · 2017-01-06T21:11:04Z

test/pkg.jl

-        ret, out, err = @grab_outputs Pkg.pin("Example", v"0.4.0")
-        @test ret === nothing && out == ""
-        @test contains(err, "INFO: Package Example is already pinned to the selected commit")
-        @test !contains(err, "INFO: No packages to install, update or remove")


don't delete

Wasn't deleted: I used r"^....$" as above to make sure only one warning is printed

tkelman · 2017-01-06T21:14:21Z

test/pkg.jl

-        @test contains(err, "INFO: Package Example: skipping update (pinned)...\n")
-        @test ismatch(r"INFO: Package Example was set to version 0\.4\.0, but a higher version \d+\.\d+\.\d+\S* exists.\n", err)
-        @test contains(err, "The package is fixed. You can try using `Pkg.free(\"Example\")` to update it.")
-        @test contains(err, nothingtodomsg)


don't delete

whoops, will re-add the nothingtodomsg check

tkelman · 2017-01-06T21:17:33Z

test/pkg.jl

+
+        @test_warn (r"INFO: Upgrading ColorTypes: v0\.2\.2 => v\d+\.\d+\.\d+",
+                    r"INFO: Upgrading Compat: v0\.7\.18 => v\d+\.\d+\.\d+",
+                    r"INFO: Upgrading Color[^s]") Pkg.update("ColorTypes")


this won't verify that Colors doesn't get upgraded

hmm, right, I need a "negative pattern" here ... is there any way to do this with just a regex?

Otherwise, I can modify @test_warn so that it accepts a boolean function, and then we can test whatever we want.

… pkg test

stevengj · 2017-01-06T21:47:08Z

@tkelman, I think I've addressed all of your comments.

stevengj · 2017-01-07T03:40:41Z

Hooray, 32-bit Travis is passing again!

tkelman · 2017-01-07T14:02:24Z

test/compile.jl

        @test isa(Base._require_from_serialized(myid(), Foo_module, cachefile, #=broadcast-load=#false), Array{Any,1})
-    finally
-        close(STDERR)
-        redirect_stderr(olderr)
    end


~~this is now misleadingly ending a much-earlier try~~

misread, didn't see the begin

tkelman · 2017-01-07T14:05:42Z

test/core.jl

@@ -4822,6 +4822,17 @@ end
 @test f14893() == 14893
 @test M14893.f14893() == 14893

+# issue #18725
+@test_nowarn begin


don't tests run in a non-Main anonymous module now?

Weird, right: the warning is getting output (correctly), but for some reason it is not captured by redirect_stderr.

For example, if I do the following in test/core.jl:

let fname = tempname() try open(fname, "w") do f redirect_stderr(f) do begin f18725x(x) = 1 f18725x(x) = 2 @test f18725x(0) == 2 end end end println("GOT WARNING: \"", readstring(fname), '"') finally rm(fname, force=true) end end

it outputs

WARNING: Method definition f18725x(Any) in module TestMain_core at /Users/stevenj/Documents/Code/julia/test/core.jl:4830 overwritten at /Users/stevenj/Documents/Code/julia/test/core.jl:4831. GOT WARNING: ""

(And yet, the other @test_warn passes, indicating that the warning is sometimes being captured by redirect_stderr)

Any idea why the warning would not be captured here?

If I change the begin...end to

eval(current_module(), :(begin f18725x(x) = 1 f18725x(x) = 2 @test f18725x(0) == 2 end)) end

then it correctly captures the warning.

Somehow, it seems that the "Method definition overwritten" error is being printed at some other stage of code evaluation, so that it occurs before the redirect_stderr is executed.

I've pushed a workaround, and verified that it is not printing any warning that fails to get captured. (The issue of when the method-override warning gets printed seems unrelated to this PR.)

(My workaround now evals the expression in Main, so it correctly gets the case that is not supposed to have a warning.)

tkelman

lgtm if passes CI and squashed on merge (or before if you want to split it to a couple separate commits)

anriseth · 2017-05-09T08:32:25Z

Is there a way to use @test_warn in 0.5?
For compatibility between 0.5 and 0.6, I have tried to do:

if isdefined(Symbol("@test_warn"))
  @test_warn msg expr
else
  expr
end

But it seems like Julia 0.5 tries to expand the macro even if isdefined(Symbol("@test_warn")) == false

tkelman · 2017-05-09T08:36:56Z

someone could contribute this to either BaseTestNext or Compat

probably need to specify isdefined(Base.Test, Symbol("@test_warn")) instead of asking about Main.

anriseth · 2017-05-09T08:42:55Z

Okay, thanks for the reply @tkelman. I'll wait with @test_warn until the package stops supporting 0.5

stevengj added the testsystem The unit testing framework and Test stdlib label Jan 6, 2017

kshyatt reviewed Jan 6, 2017

View reviewed changes

stevengj force-pushed the test_warn branch from 915db79 to 958bf0f Compare January 6, 2017 17:34

kshyatt reviewed Jan 6, 2017

View reviewed changes

base/test.jl

"""

@test_nowarn expr

Test whether evaluating `expr` results in empty `stderr` output

Copy link

Contributor

kshyatt Jan 6, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here re stderr

stevengj added 3 commits January 6, 2017 12:34

add test_warn macro to Base.Test for verifying warning messages

ee012bd

use test_warn in compilation tests

5103a1c

add test for JuliaLang#18725

63c55d0

stevengj force-pushed the test_warn branch from 958bf0f to 63c55d0 Compare January 6, 2017 17:35

doc links for test_warn

5ac63ab

kshyatt approved these changes Jan 6, 2017

View reviewed changes

stevengj added 3 commits January 6, 2017 12:43

compile test no longer needs to save STDERR

fc2ece2

rewrite pkg tests in terms of test_nowarn

89257bd

don't strip trailing newline in test_warn, don't allow trailing newli…

ee53a54

…ne in test_nowarn

check for specific branch in pkg pinning test

6febcfa

stevengj mentioned this pull request Jan 6, 2017

don't warn about redefining methods in Main #19888

Merged

stevengj added 3 commits January 6, 2017 14:51

rm extraneous parens

0d8447a

restore pkg test for lack of output

f99c96a

return something useful from test_warn

d981d39

tkelman reviewed Jan 6, 2017

View reviewed changes

stevengj added 2 commits January 6, 2017 16:35

small fixes

6ae2e67

support arbitrary boolean function in test_warn, fix Upgrading Colors…

3ca3209

… pkg test

delete blank line in docstring

3e7ca3a

tkelman reviewed Jan 7, 2017

View reviewed changes

eval 18725 in Main

28c208c

tkelman approved these changes Jan 7, 2017

View reviewed changes

stevengj merged commit 28a11ff into JuliaLang:master Jan 7, 2017

stevengj deleted the test_warn branch January 7, 2017 17:42

kalmarek mentioned this pull request Aug 22, 2017

Warning in fmpz_mat tests Nemocas/Nemo.jl#236

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add test_warn macro to Base.Test for checking for warnings #19903

Add test_warn macro to Base.Test for checking for warnings #19903

stevengj commented Jan 6, 2017

kshyatt Jan 6, 2017 •

edited

Loading

kshyatt Jan 6, 2017

kshyatt Jan 6, 2017

kshyatt left a comment

stevengj commented Jan 6, 2017 •

edited

Loading

tkelman Jan 6, 2017

stevengj Jan 6, 2017

tkelman Jan 6, 2017

stevengj Jan 6, 2017

tkelman Jan 6, 2017

stevengj Jan 6, 2017 •

edited

Loading

tkelman Jan 6, 2017

stevengj Jan 6, 2017

tkelman Jan 6, 2017

stevengj Jan 6, 2017 •

edited

Loading

stevengj commented Jan 6, 2017

stevengj commented Jan 7, 2017

tkelman Jan 7, 2017 •

edited

Loading

tkelman Jan 7, 2017

stevengj Jan 7, 2017 •

edited

Loading

stevengj Jan 7, 2017

stevengj Jan 7, 2017

stevengj Jan 7, 2017

stevengj Jan 7, 2017

tkelman left a comment •

edited

Loading

anriseth commented May 9, 2017

tkelman commented May 9, 2017

anriseth commented May 9, 2017

Add test_warn macro to Base.Test for checking for warnings #19903

Add test_warn macro to Base.Test for checking for warnings #19903

Conversation

stevengj commented Jan 6, 2017

kshyatt Jan 6, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kshyatt left a comment

Choose a reason for hiding this comment

stevengj commented Jan 6, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stevengj Jan 6, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stevengj Jan 6, 2017 • edited Loading

Choose a reason for hiding this comment

stevengj commented Jan 6, 2017

stevengj commented Jan 7, 2017

tkelman Jan 7, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stevengj Jan 7, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tkelman left a comment • edited Loading

Choose a reason for hiding this comment

anriseth commented May 9, 2017

tkelman commented May 9, 2017

anriseth commented May 9, 2017

kshyatt Jan 6, 2017 •

edited

Loading

stevengj commented Jan 6, 2017 •

edited

Loading

stevengj Jan 6, 2017 •

edited

Loading

stevengj Jan 6, 2017 •

edited

Loading

tkelman Jan 7, 2017 •

edited

Loading

stevengj Jan 7, 2017 •

edited

Loading

tkelman left a comment •

edited

Loading