use ScopedValues for TestSets #53462

simonbyrne · 2024-02-25T07:26:45Z

Another attempt at #51012

simonbyrne · 2024-02-28T05:13:39Z

@vchuravy @Keno I'm getting some weird failures on this branch originating from this line:

Line 1162 in c379db7

@assert current_mapping == val

The simplest reproducer I have is:

using Test
@testset begin
    @testset for r in Int[1]
    end
end

simonbyrne · 2024-02-29T06:09:40Z

Blocked by #53521

nickrobinson251 · 2024-09-23T19:26:29Z

Would love to see this merged!

@simonbyrne do you have any time to work on this? I'd be happy to rebase and help get it over the line. I think rebasing, and then deciding what to do with the commented-out scope tests might be all that's needed here?

stdlib/Test/src/Test.jl

giordano · 2024-09-23T19:54:05Z

I initially thought I managed to badly botch the rebase, but I believe this is now failing to build because after the rebase #55452 was merged in and that PR removed using .ScopedValues inside Base, so imports need to change a little bit. I'll leave this exercise to someone else 😁

simonbyrne · 2024-09-23T19:55:17Z

Sorry, it still needs some work. The handling of errors is wasn't quite correct, and the resetting of rng state is kind of confusing (arguably should be handled differently)

base/scopedvalues.jl

simonbyrne · 2024-10-09T04:34:02Z

Currently blocked by #56062

d-netto · 2025-01-30T15:13:25Z

FYI, #56062 was recently fixed.

simonbyrne · 2025-02-26T05:32:55Z

Okay, I think this is now passing. Any thoughts?

simonbyrne · 2025-03-03T00:18:47Z

@nanosoldier runtests()

giordano

Even though I recently touched the Test stdlib I'm not particularly familiar with its code, I don't understand some of the design choices. That said, this looks overall good to me (as much as that mean, not knowing the code), looks like a decent simplification. I'm going to ~~trust~~ hope existing tests already cover corner cases of nested testsets that we should make sure keep working.

stdlib/Test/test/runtests.jl

stdlib/Test/src/Test.jl

giordano · 2025-03-02T23:59:25Z

test/scopedvalues.jl

-    @test sprint(show, Core.current_scope()) == "nothing"
+    @test sprint(show, ScopedValue{Int}()) == "$ScopedValue{$Int}(undefined)"
+    @test sprint(show, sval) == "$ScopedValue{$Int}(1)"
+    # @test sprint(show, Core.current_scope()) == "nothing"


Why was this test commented out? Can we move it at the top-level, outside of any testset, to keep the test, or that's still within an implicit scope?

Depending on how the runtests is called, it might be inside a scope. The only way to guarantee it is to run it in a separate process.

giordano · 2025-03-02T23:59:57Z

test/scopedvalues.jl

        objid = sprint(show, Base.objectid(sval))
-        @test sprint(show, Core.current_scope()) == "Base.ScopedValues.Scope(Base.ScopedValues.ScopedValue{$Int}@$objid => 2)"
+        # @test sprint(show, Core.current_scope()) == "Base.ScopedValues.Scope(Base.ScopedValues.ScopedValue{$Int}@$objid => 2)"


Also this: why was it removed?

Same, I can't see a way to make this work.

giordano · 2025-03-03T00:35:45Z

stdlib/Test/src/Test.jl

-"""
-    push_testset(ts::AbstractTestSet)
-
-Adds the test set to the `task_local_storage`.
-"""
-function push_testset(ts::AbstractTestSet)
-    testsets = get(task_local_storage(), :__BASETESTNEXT__, AbstractTestSet[])
-    push!(testsets, ts)
-    setindex!(task_local_storage(), testsets, :__BASETESTNEXT__)
-end


Apparently some (testing) packages were using this: https://juliahub.com/ui/Search?type=symbols&q=push_testset&u=use. Presumably it's ok to ask them to adapt their use of internal functions? These aren't documented.

Yes, unfortunately I don't see how we could keep that working

How would one update a codebase that is using push_testset? is it as simple as changing from

push_testset(ts) try ... finally pop_testset() end

to

@with_testset ts begin ... end

?

(p.s. Very pleased to see this improvement!)

Yes, that should work I think.

giordano · 2025-03-03T00:37:07Z

stdlib/Test/src/Test.jl

-                get_testset_depth() > 1 ? rethrow() : failfast_print()
+                get_testset_depth() > 0 ? rethrow() : failfast_print()


I can't immediately tell the motivation for this change, could you please elaborate?

Previously the check used to be inside the scope, now it is outside

Co-authored-by: Mosè Giordano <[email protected]>

nanosoldier · 2025-03-05T18:01:07Z

The package evaluation job you requested has completed - possible new issues were detected.
The full report is available.

Report summary

❗ Packages that crashed

3 packages crashed only on the current version.

An internal error was encountered: 1 packages
A segmentation fault happened: 2 packages

14 packages crashed on the previous version too.

✖ Packages that failed

181 packages failed only on the current version.

Package fails to precompile: 19 packages
Package has test failures: 3 packages
Package tests unexpectedly errored: 133 packages
Test duration exceeded the time limit: 26 packages

1132 packages failed on the previous version too.

✔ Packages that passed tests

22 packages passed tests only on the current version.

Other: 22 packages

5158 packages passed tests on the previous version too.

~ Packages that at least loaded

18 packages successfully loaded only on the current version.

Other: 18 packages

2933 packages successfully loaded on the previous version too.

➖ Packages that were skipped altogether

897 packages were skipped on the previous version too.

simonbyrne · 2025-03-09T19:50:04Z

I think this is ready to go. @vchuravy any last thoughts?

vchuravy · 2025-03-09T20:37:28Z

ScopedValue use seems fine!

giordano · 2025-03-09T22:48:25Z

Why is llvmpasses failing on this PR? That sounds surprising https://buildkite.com/julialang/julia-master/builds/45612#01957cc9-70e1-4ef5-a7e5-f3aceedce57d/1209-1246

FAIL: Julia :: image-codegen.jl (30 of 44)
******************** TEST 'Julia :: image-codegen.jl' FAILED ********************
Exit Code: 1
Command Output (stderr):
--
RUN: at line 2: export JULIA_LLVM_ARGS="--print-before=loop-vectorize --print-module-scope"
+ export 'JULIA_LLVM_ARGS=--print-before=loop-vectorize --print-module-scope'
+ JULIA_LLVM_ARGS='--print-before=loop-vectorize --print-module-scope'
RUN: at line 3: rm -rf /cache/build/builder-amdci5-1/julialang/julia-master/test/llvmpasses/Output/image-codegen.jl.tmp
+ rm -rf /cache/build/builder-amdci5-1/julialang/julia-master/test/llvmpasses/Output/image-codegen.jl.tmp
RUN: at line 4: mkdir /cache/build/builder-amdci5-1/julialang/julia-master/test/llvmpasses/Output/image-codegen.jl.tmp
+ mkdir /cache/build/builder-amdci5-1/julialang/julia-master/test/llvmpasses/Output/image-codegen.jl.tmp
RUN: at line 5: julia --image-codegen --startup-file=no /cache/build/builder-amdci5-1/julialang/julia-master/test/llvmpasses/image-codegen.jl 2> /cache/build/builder-amdci5-1/julialang/julia-master/test/llvmpasses/Output/image-codegen.jl.tmp/output.txt
+ julia --image-codegen --startup-file=no /cache/build/builder-amdci5-1/julialang/julia-master/test/llvmpasses/image-codegen.jl
RUN: at line 6: FileCheck /cache/build/builder-amdci5-1/julialang/julia-master/test/llvmpasses/image-codegen.jl < /cache/build/builder-amdci5-1/julialang/julia-master/test/llvmpasses/Output/image-codegen.jl.tmp/output.txt
+ FileCheck /cache/build/builder-amdci5-1/julialang/julia-master/test/llvmpasses/image-codegen.jl
/cache/build/builder-amdci5-1/julialang/julia-master/test/llvmpasses/image-codegen.jl:17:15: error: CHECK-SAME: is not on the same line as the previous match
# CHECK-SAME: = {{(external )?}}
              ^
<stdin>:89:5: note: 'next' match was here
 %7 = addrspacecast ptr %6 to ptr addrspace(10), !dbg !16
    ^
<stdin>:88:32: note: previous match ended here
 %6 = load ptr, ptr @"jl_global#5", align 8, !dbg !16, !tbaa !13, !invariant.load !0, !alias.scope !17, !noalias !20, !nonnull !0
                               ^
Input file: <stdin>
Check file: /cache/build/builder-amdci5-1/julialang/julia-master/test/llvmpasses/image-codegen.jl
-dump-input=help explains the following input dump.
Input was:
<<<<<<
         .
         .
         .
        84: %4, align 8, !tbaa !13, !invariant.load !0
        85:  fence syncscope("singlethread") seq_cst
        86:  call void @julia.safepoint(ptr %5), !dbg !15
        87:  fence syncscope("singlethread") seq_cst
        88:  %6 = load ptr, ptr @"jl_global#5", align 8, !dbg !16, !tbaa !13, !invariant.load !0, !alias.scope !17, !noalias !20, !nonnull !0
        89:  %7 = addrspacecast ptr %6 to ptr addrspace(10), !dbg !16
same:17         !~                                                     error: match on wrong line
        90:  ret ptr addrspace(10) %7, !dbg !16
        91: }
        92:
        93: ; Function Attrs: noinline optnone
        94: define nonnull ptr addrspace(10) @jfptr_f_4(ptr addrspace(10) %0, ptr noalias nocapture noundef readonly %1, i32 %2) local_unnamed_addr #1 {
         .
         .
         .
>>>>>>
--
********************

simonbyrne force-pushed the sb/test-scope branch 6 times, most recently from 1c67443 to b1fec32 Compare February 27, 2024 05:38

simonbyrne force-pushed the sb/test-scope branch 2 times, most recently from 73b39d1 to 0cd361c Compare March 9, 2024 01:00

simonbyrne force-pushed the sb/test-scope branch from 0cd361c to 7ef5878 Compare March 17, 2024 16:41

simonbyrne added 2 commits May 25, 2024 10:42

use ScopedValues for TestSets

34b945c

comment out broken tests

7849324

simonbyrne force-pushed the sb/test-scope branch from 7ef5878 to 7849324 Compare May 25, 2024 17:42

giordano added testsystem The unit testing framework and Test stdlib stdlib Julia's standard library labels Sep 23, 2024

Merge branch 'master' into sb/test-scope

bd44c2a

giordano reviewed Sep 23, 2024

View reviewed changes

stdlib/Test/src/Test.jl Outdated Show resolved Hide resolved

Update stdlib/Test/src/Test.jl

b9d7ab8

simonbyrne added 4 commits September 23, 2024 22:10

fix imports

dcf2977

add extra conversions, tweak tests

e664d5b

more fixes

5097261

Merge remote-tracking branch 'origin/master' into sb/test-scope

0a02d40

simonbyrne marked this pull request as ready for review October 5, 2024 03:51

simonbyrne added 3 commits October 5, 2024 07:42

Merge remote-tracking branch 'origin/master' into sb/test-scope

a645b3b

fix doctest

19706fd

don't reverse

e083c75

IanButterworth mentioned this pull request Oct 7, 2024

Fix no-arg ScopedValues.@with within a scope #56019

Merged

IanButterworth reviewed Oct 7, 2024

View reviewed changes

base/scopedvalues.jl Outdated Show resolved Hide resolved

Merge remote-tracking branch 'origin/master' into sb/test-scope

cd3f239

Merge branch 'master' into sb/test-scope

449a7ab

simonbyrne force-pushed the sb/test-scope branch from 1a89ef6 to 449a7ab Compare January 31, 2025 17:28

simonbyrne added 3 commits February 3, 2025 18:55

fix more errors

6f7625c

Merge branch 'master' into sb/test-scope

15f609e

Remove duplicate definition

490ee21

simonbyrne requested review from giordano and vchuravy February 26, 2025 05:33

giordano reviewed Mar 3, 2025

View reviewed changes

simonbyrne and others added 2 commits March 2, 2025 17:50

Use addenv

b43e9a4

Co-authored-by: Mosè Giordano <[email protected]>

Remove comment

dddc4ed

simonbyrne added 2 commits March 9, 2025 12:36

Merge remote-tracking branch 'origin/master' into sb/test-scope

47fc0c9

remove scopedvalue tests that no longer work

9f8088a

vchuravy added the needs news A NEWS entry is required for this change label Mar 10, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

use ScopedValues for TestSets #53462

use ScopedValues for TestSets #53462

simonbyrne commented Feb 25, 2024

simonbyrne commented Feb 28, 2024

simonbyrne commented Feb 29, 2024

nickrobinson251 commented Sep 23, 2024

giordano commented Sep 23, 2024

simonbyrne commented Sep 23, 2024

simonbyrne commented Oct 9, 2024

d-netto commented Jan 30, 2025

simonbyrne commented Feb 26, 2025

simonbyrne commented Mar 3, 2025

giordano left a comment

giordano Mar 2, 2025

simonbyrne Mar 3, 2025

giordano Mar 2, 2025

simonbyrne Mar 9, 2025

giordano Mar 3, 2025

simonbyrne Mar 3, 2025

nickrobinson251 Mar 9, 2025

simonbyrne Mar 10, 2025

giordano Mar 3, 2025

simonbyrne Mar 3, 2025

nanosoldier commented Mar 5, 2025

❗ Packages that crashed

✖ Packages that failed

✔ Packages that passed tests

~ Packages that at least loaded

➖ Packages that were skipped altogether

simonbyrne commented Mar 9, 2025

vchuravy commented Mar 9, 2025

giordano commented Mar 9, 2025

		get_testset_depth() > 1 ? rethrow() : failfast_print()
		get_testset_depth() > 0 ? rethrow() : failfast_print()

use ScopedValues for TestSets #53462

Are you sure you want to change the base?

use ScopedValues for TestSets #53462

Conversation

simonbyrne commented Feb 25, 2024

simonbyrne commented Feb 28, 2024

simonbyrne commented Feb 29, 2024

nickrobinson251 commented Sep 23, 2024

giordano commented Sep 23, 2024

simonbyrne commented Sep 23, 2024

simonbyrne commented Oct 9, 2024

d-netto commented Jan 30, 2025

simonbyrne commented Feb 26, 2025

simonbyrne commented Mar 3, 2025

giordano left a 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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nanosoldier commented Mar 5, 2025

❗ Packages that crashed

✖ Packages that failed

✔ Packages that passed tests

~ Packages that at least loaded

➖ Packages that were skipped altogether

simonbyrne commented Mar 9, 2025

vchuravy commented Mar 9, 2025

giordano commented Mar 9, 2025