Revert back to our hand-rolled test runner. #658
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Some suggestions could not be made:
- test/bpf.jl
- lines 4-7
- lines 29-29
- test/examples.jl
- lines 1-1
- lines 9-9
- lines 21-21
- test/gcn.jl
- lines 3-12
- lines 20-59
- lines 78-130
- lines 172-208
- test/helpers/bpf.jl
- lines 7-13
- test/helpers/gcn.jl
- lines 7-17
- test/helpers/metal.jl
- lines 7-17
- test/helpers/native.jl
- lines 13-22
- lines 64-65
- lines 73-76
- test/helpers/precompile.jl
- lines 2-2
- lines 15-16
- lines 29-29
- lines 59-59
- test/helpers/ptx.jl
- lines 8-8
- lines 38-50
- test/helpers/runtime.jl
- lines 2-8
- test/helpers/spirv.jl
- lines 7-17
- test/helpers/test.jl
- lines 3-3
- lines 22-29
- test/metal.jl
- lines 3-121
- test/native.jl
- lines 9-14
- lines 40-44
- lines 73-77
- lines 110-110
- lines 127-127
- lines 136-136
- lines 151-151
- lines 163-163
- lines 177-252
- lines 289-346
- lines 373-447
- lines 493-514
- lines 542-622
- test/ptx.jl
- lines 3-102
- lines 130-133
- lines 143-148
- lines 159-191
- lines 205-255
- lines 300-336
- lines 351-397
- test/runtests.jl
- lines 392-393
- test/spirv.jl
- lines 3-66
- test/utils.jl
- lines 2-5
- lines 31-31
- lines 40-43
Comment on lines
+8
to
+9
| function extract_flag!(args, flag, default=nothing) | ||
| for f in args |
There was a problem hiding this comment.
Suggested change
| function extract_flag!(args, flag, default=nothing) | |
| for f in args | |
| function extract_flag!(args, flag, default = nothing) | |
| val = parse(typeof(default), val) |
| end | ||
| do_help, _ = extract_flag!(ARGS, "--help") | ||
| if do_help | ||
| println(""" |
There was a problem hiding this comment.
Suggested change
| println(""" | |
| println( | |
| """ |
| --quickfail Fail the entire run as soon as a single test errored. | ||
| --jobs=N Launch `N` processes to perform tests (default: Sys.CPU_THREADS). | ||
|
|
||
| Remaining arguments filter the tests that will be executed.""") |
There was a problem hiding this comment.
Suggested change
| Remaining arguments filter the tests that will be executed.""") | |
| Remaining arguments filter the tests that will be executed.""" | |
| ) |
Comment on lines
+53
to
+76
| # find Julia files | ||
| filter!(files) do file | ||
| endswith(file, ".jl") && file !== "setup.jl" && file !== "runtests.jl" | ||
| end | ||
| isempty(files) && continue | ||
| basename(rootpath) == "helpers" && continue | ||
|
|
||
| # strip extension | ||
| files = map(files) do file | ||
| file[1:end-3] | ||
| end | ||
|
|
||
| # prepend subdir | ||
| subdir = relpath(rootpath, @__DIR__) | ||
| if subdir != "." | ||
| files = map(files) do file | ||
| joinpath(subdir, file) | ||
| end | ||
| end | ||
|
|
||
| @dispose ctx=Context() begin | ||
| # XXX: some back-ends do not support opaque pointers | ||
| if ti.name in ["Metal"] && !supports_typed_pointers(ctx) | ||
| return false | ||
| end | ||
| # unify path separators | ||
| files = map(files) do file | ||
| replace(file, path_separator => '/') | ||
| end |
There was a problem hiding this comment.
Suggested change
| # find Julia files | |
| filter!(files) do file | |
| endswith(file, ".jl") && file !== "setup.jl" && file !== "runtests.jl" | |
| end | |
| isempty(files) && continue | |
| basename(rootpath) == "helpers" && continue | |
| # strip extension | |
| files = map(files) do file | |
| file[1:end-3] | |
| end | |
| # prepend subdir | |
| subdir = relpath(rootpath, @__DIR__) | |
| if subdir != "." | |
| files = map(files) do file | |
| joinpath(subdir, file) | |
| end | |
| end | |
| @dispose ctx=Context() begin | |
| # XXX: some back-ends do not support opaque pointers | |
| if ti.name in ["Metal"] && !supports_typed_pointers(ctx) | |
| return false | |
| end | |
| # unify path separators | |
| files = map(files) do file | |
| replace(file, path_separator => '/') | |
| end | |
| # find Julia files | |
| filter!(files) do file | |
| endswith(file, ".jl") && file !== "setup.jl" && file !== "runtests.jl" | |
| end | |
| isempty(files) && continue | |
| basename(rootpath) == "helpers" && continue | |
| # strip extension | |
| file[1:(end - 3)] | |
| # prepend subdir | |
| subdir = relpath(rootpath, @__DIR__) | |
| if subdir != "." | |
| files = map(files) do file | |
| joinpath(subdir, file) | |
| end | |
| end | |
| # unify path separators | |
| files = map(files) do file | |
| replace(file, path_separator => '/') | |
| end | |
| append!(all_tests, files) | |
| for file in files | |
| test_runners[file] = () -> include("$(@__DIR__)/$file.jl") | |
| end |
Comment on lines
+102
to
+104
| filter!(tests) do test | ||
| any(arg->startswith(test, arg), ARGS) | ||
| end |
There was a problem hiding this comment.
Suggested change
| filter!(tests) do test | |
| any(arg->startswith(test, arg), ARGS) | |
| end | |
| filter!(tests) do test | |
| any(arg -> startswith(test, arg), ARGS) | |
| end |
Comment on lines
+309
to
+312
| foreach(task -> begin | ||
| istaskstarted(task) || return | ||
| istaskdone(task) && return | ||
| try |
There was a problem hiding this comment.
Suggested change
| foreach(task -> begin | |
| istaskstarted(task) || return | |
| istaskdone(task) && return | |
| try | |
| foreach( | |
| task -> begin | |
| schedule(task, InterruptException(); error = true) | |
| @error "InterruptException" exception = ex, catch_backtrace() | |
| end, all_tasks | |
| ) |
Comment on lines
+329
to
+330
| schedule(stdin_monitor, InterruptException(); error=true) | ||
| end |
There was a problem hiding this comment.
Suggested change
| schedule(stdin_monitor, InterruptException(); error=true) | |
| end | |
| schedule(stdin_monitor, InterruptException(); error = true) | |
| elapsed = canonicalize(Dates.CompoundPeriod(t1 - t0)) |
| Test.push_testset(resp) | ||
| Test.record(o_ts, resp) | ||
| Test.pop_testset() | ||
| elseif isa(resp, Tuple{Int,Int}) |
There was a problem hiding this comment.
Suggested change
| elseif isa(resp, Tuple{Int,Int}) | |
| elseif isa(resp, Tuple{Int, Int}) |
| old_print_setting = Test.TESTSET_PRINT_ENABLE[] | ||
| Test.TESTSET_PRINT_ENABLE[] = false | ||
|
|
||
| try |
There was a problem hiding this comment.
Suggested change
| try | |
| return try |
Comment on lines
+45
to
+51
| passes,fails,error,broken,c_passes,c_fails,c_errors,c_broken = | ||
| tc.passes, tc.fails, tc.errors, tc.broken, tc.cumulative_passes, | ||
| tc.cumulative_fails, tc.cumulative_errors, tc.cumulative_broken | ||
| else | ||
| passes,fails,errors,broken,c_passes,c_fails,c_errors,c_broken = | ||
| Test.get_test_counts(data[1]) | ||
| end |
There was a problem hiding this comment.
Suggested change
| passes,fails,error,broken,c_passes,c_fails,c_errors,c_broken = | |
| tc.passes, tc.fails, tc.errors, tc.broken, tc.cumulative_passes, | |
| tc.cumulative_fails, tc.cumulative_errors, tc.cumulative_broken | |
| else | |
| passes,fails,errors,broken,c_passes,c_fails,c_errors,c_broken = | |
| Test.get_test_counts(data[1]) | |
| end | |
| passes, fails, error, broken, c_passes, c_fails, c_errors, c_broken = | |
| passes, fails, errors, broken, c_passes, c_fails, c_errors, c_broken = | |
| data = ( | |
| (passes + c_passes, broken + c_broken), | |
| data[2], | |
| data[3], | |
| data[4], | |
| data[5], | |
| ) |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #658 +/- ##
==========================================
- Coverage 73.69% 72.11% -1.59%
==========================================
Files 24 24
Lines 3334 3310 -24
==========================================
- Hits 2457 2387 -70
- Misses 877 923 +46 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Some suggestions could not be made:
- test/bpf.jl
- lines 4-7
- lines 29-29
- test/examples.jl
- lines 1-1
- lines 9-9
- lines 21-21
- test/gcn.jl
- lines 3-12
- lines 20-59
- lines 78-130
- lines 172-208
- test/helpers/bpf.jl
- lines 7-13
- test/helpers/gcn.jl
- lines 7-17
- test/helpers/metal.jl
- lines 7-17
- test/helpers/native.jl
- lines 13-22
- lines 64-65
- lines 73-76
- test/helpers/precompile.jl
- lines 10-11
- lines 20-20
- lines 50-50
- test/helpers/ptx.jl
- lines 8-8
- lines 38-50
- test/helpers/runtime.jl
- lines 2-8
- test/helpers/spirv.jl
- lines 7-17
- test/helpers/test.jl
- lines 3-3
- lines 22-29
- test/metal.jl
- lines 3-121
- test/native.jl
- lines 9-14
- lines 40-44
- lines 73-77
- lines 110-110
- lines 127-127
- lines 136-136
- lines 151-151
- lines 163-163
- lines 177-252
- lines 289-346
- lines 373-447
- lines 493-514
- lines 542-553
- test/ptx.jl
- lines 3-102
- lines 130-133
- lines 143-148
- lines 159-191
- lines 205-255
- lines 300-336
- test/runtests.jl
- lines 395-396
- test/spirv.jl
- lines 3-66
- test/utils.jl
- lines 2-5
- lines 31-31
- lines 40-43
| using ReTestItems | ||
|
|
||
| export precompile_test_harness, check_presence, create_standalone | ||
|
|
||
| function precompile_test_harness(@nospecialize(f), testset::String) | ||
| @testset "$testset" begin |
There was a problem hiding this comment.
Suggested change
| @testset "$testset" begin | |
| return @testset "$testset" begin |
Comment on lines
+7
to
+26
| write(joinpath(load_path, "NativeBackend.jl"), :( | ||
| module NativeBackend | ||
| import NativeCompiler | ||
| using PrecompileTools | ||
|
|
||
| function kernel(A, x) | ||
| A[1] = x | ||
| return | ||
| end | ||
|
|
||
| let | ||
| job, _ = NativeCompiler.Native.create_job(kernel, (Vector{Int}, Int)) | ||
| precompile(job) | ||
| end | ||
|
|
||
| # identity is foreign | ||
| @setup_workload begin | ||
| job, _ = NativeCompiler.Native.create_job(identity, (Int,)) | ||
| @compile_workload begin | ||
| precompile(job) |
There was a problem hiding this comment.
Suggested change
| write(joinpath(load_path, "NativeBackend.jl"), :( | |
| module NativeBackend | |
| import NativeCompiler | |
| using PrecompileTools | |
| function kernel(A, x) | |
| A[1] = x | |
| return | |
| end | |
| let | |
| job, _ = NativeCompiler.Native.create_job(kernel, (Vector{Int}, Int)) | |
| precompile(job) | |
| end | |
| # identity is foreign | |
| @setup_workload begin | |
| job, _ = NativeCompiler.Native.create_job(identity, (Int,)) | |
| @compile_workload begin | |
| precompile(job) | |
| write( | |
| joinpath(load_path, "NativeBackend.jl"), :( | |
| module NativeBackend | |
| import NativeCompiler | |
| using PrecompileTools | |
| function kernel(A, x) | |
| A[1] = x | |
| return | |
| end | |
| let | |
| job, _ = NativeCompiler.Native.create_job(kernel, (Vector{Int}, Int)) | |
| # identity is foreign | |
| @setup_workload begin | |
| job, _ = NativeCompiler.Native.create_job(identity, (Int,)) | |
| @compile_workload begin | |
| precompile(job) | |
| end | |
| end | |
| end | |
| ) |> string | |
| ) |
| @test check_presence(kernel_mi, token) | ||
|
|
||
| # check that identity survived | ||
| @test check_presence(identity_mi, token) broken=VERSION>=v"1.12.0-DEV.1268" |
There was a problem hiding this comment.
Suggested change
| @test check_presence(identity_mi, token) broken=VERSION>=v"1.12.0-DEV.1268" | |
| @test check_presence(identity_mi, token) broken = VERSION >= v"1.12.0-DEV.1268" |
Comment on lines
+5
to
+23
| write(joinpath(load_path, "PTXBackend.jl"), :( | ||
| module PTXBackend | ||
| import PTXCompiler | ||
| using PrecompileTools | ||
|
|
||
| function kernel() | ||
| return | ||
| end | ||
|
|
||
| let | ||
| job, _ = PTXCompiler.PTX.create_job(kernel, ()) | ||
| precompile(job) | ||
| end | ||
|
|
||
| # identity is foreign | ||
| @setup_workload begin | ||
| job, _ = PTXCompiler.PTX.create_job(identity, (Int,)) | ||
| @compile_workload begin | ||
| precompile(job) |
There was a problem hiding this comment.
Suggested change
| write(joinpath(load_path, "PTXBackend.jl"), :( | |
| module PTXBackend | |
| import PTXCompiler | |
| using PrecompileTools | |
| function kernel() | |
| return | |
| end | |
| let | |
| job, _ = PTXCompiler.PTX.create_job(kernel, ()) | |
| precompile(job) | |
| end | |
| # identity is foreign | |
| @setup_workload begin | |
| job, _ = PTXCompiler.PTX.create_job(identity, (Int,)) | |
| @compile_workload begin | |
| precompile(job) | |
| write( | |
| joinpath(load_path, "PTXBackend.jl"), :( | |
| module PTXBackend | |
| import PTXCompiler | |
| using PrecompileTools | |
| function kernel() | |
| return | |
| end | |
| let | |
| job, _ = PTXCompiler.PTX.create_job(kernel, ()) | |
| # identity is foreign | |
| @setup_workload begin | |
| job, _ = PTXCompiler.PTX.create_job(identity, (Int,)) | |
| @compile_workload begin | |
| precompile(job) | |
| end | |
| end | |
| end | |
| ) |> string | |
| ) |
| @test check_presence(kernel_mi, token) | ||
|
|
||
| # check that identity survived | ||
| @test check_presence(identity_mi, token) broken=VERSION>=v"1.12.0-DEV.1268" |
There was a problem hiding this comment.
Suggested change
| @test check_presence(identity_mi, token) broken=VERSION>=v"1.12.0-DEV.1268" | |
| @test check_presence(identity_mi, token) broken = VERSION >= v"1.12.0-DEV.1268" |
Comment on lines
+312
to
+315
| foreach(task -> begin | ||
| istaskstarted(task) || return | ||
| istaskdone(task) && return | ||
| try |
There was a problem hiding this comment.
Suggested change
| foreach(task -> begin | |
| istaskstarted(task) || return | |
| istaskdone(task) && return | |
| try | |
| foreach( | |
| task -> begin | |
| schedule(task, InterruptException(); error = true) | |
| @error "InterruptException" exception = ex, catch_backtrace() | |
| end, all_tasks | |
| ) |
Comment on lines
+332
to
+333
| schedule(stdin_monitor, InterruptException(); error=true) | ||
| end |
There was a problem hiding this comment.
Suggested change
| schedule(stdin_monitor, InterruptException(); error=true) | |
| end | |
| schedule(stdin_monitor, InterruptException(); error = true) | |
| elapsed = canonicalize(Dates.CompoundPeriod(t1 - t0)) |
| Test.push_testset(resp) | ||
| Test.record(o_ts, resp) | ||
| Test.pop_testset() | ||
| elseif isa(resp, Tuple{Int,Int}) |
There was a problem hiding this comment.
Suggested change
| elseif isa(resp, Tuple{Int,Int}) | |
| elseif isa(resp, Tuple{Int, Int}) |
| old_print_setting = Test.TESTSET_PRINT_ENABLE[] | ||
| Test.TESTSET_PRINT_ENABLE[] = false | ||
|
|
||
| try |
There was a problem hiding this comment.
Suggested change
| try | |
| return try |
Comment on lines
+45
to
+51
| passes,fails,error,broken,c_passes,c_fails,c_errors,c_broken = | ||
| tc.passes, tc.fails, tc.errors, tc.broken, tc.cumulative_passes, | ||
| tc.cumulative_fails, tc.cumulative_errors, tc.cumulative_broken | ||
| else | ||
| passes,fails,errors,broken,c_passes,c_fails,c_errors,c_broken = | ||
| Test.get_test_counts(data[1]) | ||
| end |
There was a problem hiding this comment.
Suggested change
| passes,fails,error,broken,c_passes,c_fails,c_errors,c_broken = | |
| tc.passes, tc.fails, tc.errors, tc.broken, tc.cumulative_passes, | |
| tc.cumulative_fails, tc.cumulative_errors, tc.cumulative_broken | |
| else | |
| passes,fails,errors,broken,c_passes,c_fails,c_errors,c_broken = | |
| Test.get_test_counts(data[1]) | |
| end | |
| passes, fails, error, broken, c_passes, c_fails, c_errors, c_broken = | |
| passes, fails, errors, broken, c_passes, c_fails, c_errors, c_broken = | |
| data = ( | |
| (passes + c_passes, broken + c_broken), | |
| data[2], | |
| data[3], | |
| data[4], | |
| data[5], | |
| ) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I became annoyed by ReTestItems.jl being clunky, resulting in ugly output, and breaking my workflow of being able to run tests in isolation, so let's go back to our trusty old JuliaGPU flavored test runner.
Before:
After:
A bit slower -- I guess we're doing too much on each worker by always including
setup.jl-- but not really a deal breaker.