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

Scoping issue in @testset #17170

Closed
ranjanan opened this issue Jun 28, 2016 · 11 comments
Closed

Scoping issue in @testset #17170

ranjanan opened this issue Jun 28, 2016 · 11 comments
Labels
bug Indicates an unexpected problem or unintended behavior test This change adds or pertains to unit tests

Comments

@ranjanan
Copy link
Contributor

The following piece of code:

using Base.Dates
using Base.Test
@testset "blah" begin
a = Date(2013)
@show a, "before"
function testlengths(n)
    a = Dates.Date(2000)
    for i = 1:n
        @test length(range(a,i)) == i
    end
    return a+Dates.Day(n)
end
testlengths(100000)
@show a, "after"
end

gives the output:

(a,"before") = (2013-01-01,"before")
(a,"after") = (2000-01-01,"after")

Notice that a has changed and shouldn't.

@kshyatt kshyatt added bug Indicates an unexpected problem or unintended behavior test This change adds or pertains to unit tests labels Jun 28, 2016
@StefanKarpinski
Copy link
Member

StefanKarpinski commented Jun 28, 2016

Nice MCVE.

@yuyichao
Copy link
Contributor

AFAIK, this is the expected behavior in a local scope. So should @testset be changed to not introduce a new scope?

@StefanKarpinski
Copy link
Member

Test sets introduce a new scope by design since, like try/catch they introduce rollback points. In other words, if a test in a test set fails, the tests will continue after the test set. This allows us to get much more complete testing information even when some tests fail – since the tests can continue after a failure.

@yuyichao
Copy link
Contributor

In another word this is a won't fix or dup of #14948 ?

@StefanKarpinski
Copy link
Member

I'm not sure how this is a dup of #14948 since there are no threads involved, but I do think we need to decide if this is the right behavior for test sets and it does perhaps shed some light on one of the more potentially confusing corners of Julia's scoping rules. @JeffBezanson may be interested.

@yuyichao
Copy link
Contributor

#14948 is not really about threading but the scope lifting due to a variable with the same name in the outer scope

@JeffBezanson
Copy link
Member

Chalk up another point for requiring outer (or similar keyword) to assign a variable in an outer scope, I'd have to say.

@StefanKarpinski
Copy link
Member

That seems like it may well make the scoping rules simpler and clearer.

@mauro3
Copy link
Contributor

mauro3 commented Jun 29, 2016

X-refs: #5331, #10559, #14959, #16727, #14959

@bicycle1885
Copy link
Member

I also encountered the problem. This case may be much harder to notice:

using Base.Test

@testset begin
    @testset "foo" begin
        sum = 1
    end

    @testset "bar" begin
        mktemp() do path, io
            @test sum([1,2,3]) == 6
        end
    end
end

@KristofferC
Copy link
Member

Original example now warns and the second one works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates an unexpected problem or unintended behavior test This change adds or pertains to unit tests
Projects
None yet
Development

No branches or pull requests

8 participants