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

Fix or suppress some noisy tests πŸŒοΈβ€β™‚οΈ #44444

Merged
merged 23 commits into from
Mar 6, 2022

Conversation

IanButterworth
Copy link
Sponsor Member

The noise from these in the CI logs seemed unnecessary

@DilumAluthge
Copy link
Member

Are you saying we should forgo printing the output from these tests?

@timholy
Copy link
Sponsor Member

timholy commented Mar 4, 2022

Are you saying we should forgo printing the output from these tests?

The large majority of tests are silent, and that's good because when there is a problem it's easy to find in the log.

Thanks for taming the stragglers!

@DilumAluthge
Copy link
Member

I worry that my comment about FORgoing the output was perhaps too subtle.

@DilumAluthge
Copy link
Member

Or should I say, FOURgoing?

test/backtrace.jl Outdated Show resolved Hide resolved
test/backtrace.jl Outdated Show resolved Hide resolved
@IanButterworth
Copy link
Sponsor Member Author

@timholy is this one noise? I'm guessing not because the test that should trigger that catches the log, so this seems like it's an error?

loading                                  (2) |        started at 2022-03-04T07:36:51.142
      From worker 2:	οΏ½οΏ½οΏ½ Error: active project callback #19 failed
      From worker 2:	οΏ½οΏ½οΏ½ @ Base initdefs.jl:325

@timholy
Copy link
Sponsor Member

timholy commented Mar 4, 2022

It is noise. That said, we could also remove the deliberately-errorring callback earlier in the test. See

julia/test/loading.jl

Lines 229 to 236 in b63ae3b

push!(Base.active_project_callbacks, () -> watcher_counter[] += 1)
push!(Base.active_project_callbacks, () -> error("broken"))
push!(empty!(LOAD_PATH), joinpath(@__DIR__, "project"))
append!(empty!(DEPOT_PATH), [mktempdir(), joinpath(@__DIR__, "depot")])
@test watcher_counter[] == 0
@test_logs (:error, r"active project callback .* failed") Base.set_active_project(nothing)
@test watcher_counter[] == 1
and
for _ = 1:2 pop!(Base.active_project_callbacks) end

You could pop! the second callback off after the first block, as long as you get rid of the loop in the second block.

Thanks again for doing this!

@IanButterworth IanButterworth changed the title Suppress some noisy tests πŸŒοΈβ€β™‚οΈ Fix or suppress some noisy tests πŸŒοΈβ€β™‚οΈ Mar 5, 2022
@IanButterworth
Copy link
Sponsor Member Author

IanButterworth commented Mar 5, 2022

Ok, I think this is ready to go.

There's room for improvement in the way the logs are suppressed in Pidfile, as I ended up just using a NullLogger because I couldn't figure out which lines were causing the logs, such that a @test_logs could be used. Perhaps you have a better idea of the locations @vtjnash (I added a TODO)

I marked this for 1.8 backport, but it will need manual intervention because Pidfile isn't on 1.8

@IanButterworth IanButterworth added the backport 1.8 Change should be backported to release-1.8 label Mar 5, 2022
@IanButterworth IanButterworth removed the merge me PR is reviewed. Merge when all tests are passing label Mar 5, 2022
@IanButterworth
Copy link
Sponsor Member Author

I just found another case in LibGit2 where Base.runtests wasn't able to intercept the printing of testset reports because they were generated in a subprocess. Changing them to begin blocks is a simple fix, otherwise each test file would need to know how to suppress them.

@IanButterworth
Copy link
Sponsor Member Author

IanButterworth commented Mar 5, 2022

This has grown a bit and is now in a place where I believe all unnecessary noise is fixed or suppressed, with the help of:

Given its size I'd appreciate a review, so requested you @timholy if you don't mind

Copy link
Sponsor Member

@timholy timholy left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for doing this, we'll all appreciate it.

stdlib/FileWatching/test/pidfile.jl Outdated Show resolved Hide resolved
test/boundscheck_exec.jl Show resolved Hide resolved
test/loading.jl Show resolved Hide resolved
test/misc.jl Outdated Show resolved Hide resolved
@IanButterworth IanButterworth added the merge me PR is reviewed. Merge when all tests are passing label Mar 6, 2022
@IanButterworth IanButterworth merged commit d971465 into JuliaLang:master Mar 6, 2022
@IanButterworth IanButterworth deleted the ib/denoise_tests branch March 6, 2022 12:59
@DilumAluthge DilumAluthge removed the merge me PR is reviewed. Merge when all tests are passing label Mar 6, 2022
@KristofferC KristofferC mentioned this pull request Mar 7, 2022
47 tasks
IanButterworth added a commit that referenced this pull request Mar 7, 2022
@KristofferC KristofferC removed the backport 1.8 Change should be backported to release-1.8 label Mar 15, 2022
vtjnash added a commit that referenced this pull request Apr 15, 2022
This warning was un-ironically introduced by "Fix or suppress some noisy tests".
```
β”Œ Error: mktemp cleanup
β”‚   exception =
β”‚    IOError: unlink("C:\\Windows\\TEMP\\jl_A49B.tmp"): resource busy or locked (EBUSY)
β”‚    Stacktrace:
β”‚      [1] uv_error
β”‚        @ .\libuv.jl:100 [inlined]
β”‚      [2] unlink(p::String)
β”‚        @ Base.Filesystem .\file.jl:968
β”‚      [3] rm(path::String; force::Bool, recursive::Bool)
β”‚        @ Base.Filesystem .\file.jl:283
β”‚      [4] rm
β”‚        @ .\file.jl:274 [inlined]
β”‚      [5] mktemp(fn::Main.Test49Main_misc.var"#110#113", parent::String)
β”‚        @ Base.Filesystem .\file.jl:736
β”‚      [6] mktemp(fn::Function)
β”‚        @ Base.Filesystem .\file.jl:730
β”‚      [7] macro expansion
β”‚        @ C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\test\misc.jl:1065 [inlined]
β”‚      [8] macro expansion
β”‚        @ C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\stdlib\v1.9\Test\src\Test.jl:1360 [inlined]
β”‚      [9] top-level scope
β”‚        @ C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\test\misc.jl:1060
β”‚     [10] include
β”‚        @ .\Base.jl:427 [inlined]
β”‚     [11] macro expansion
β”‚        @ C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\test\testdefs.jl:24 [inlined]
β”‚     [12] macro expansion
β”‚        @ C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\stdlib\v1.9\Test\src\Test.jl:1360 [inlined]
β”‚     [13] macro expansion
β”‚        @ C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\test\testdefs.jl:23 [inlined]
β”‚     [14] macro expansion
β”‚        @ .\timing.jl:440 [inlined]
β”‚     [15] runtests(name::String, path::String, isolate::Bool; seed::UInt128)
β”‚        @ Main C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\test\testdefs.jl:21
β”‚     [16] invokelatest(::Any, ::Any, ::Vararg{Any}; kwargs::Base.Pairs{Symbol, UInt128, Tuple{Symbol}, NamedTuple{(:seed,), Tuple{UInt128}}})
β”‚        @ Base .\essentials.jl:798
β”‚     [17] (::Distributed.var"#110#112"{Distributed.CallMsg{:call_fetch}})()
β”‚        @ Distributed C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\stdlib\v1.9\Distributed\src\process_messages.jl:285
β”‚     [18] run_work_thunk(thunk::Distributed.var"#110#112"{Distributed.CallMsg{:call_fetch}}, print_error::Bool)
β”‚        @ Distributed C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\stdlib\v1.9\Distributed\src\process_messages.jl:70
β”‚     [19] macro expansion
β”‚        @ C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\stdlib\v1.9\Distributed\src\process_messages.jl:285 [inlined]
β”‚     [20] (::Distributed.var"#109#111"{Distributed.CallMsg{:call_fetch}, Distributed.MsgHeader, Sockets.TCPSocket})()
β”‚        @ Distributed .\task.jl:490
β”” @ Base.Filesystem file.jl:738
```
giordano pushed a commit that referenced this pull request Apr 16, 2022
This warning was un-ironically introduced by "Fix or suppress some noisy tests".
```
β”Œ Error: mktemp cleanup
β”‚   exception =
β”‚    IOError: unlink("C:\\Windows\\TEMP\\jl_A49B.tmp"): resource busy or locked (EBUSY)
β”‚    Stacktrace:
β”‚      [1] uv_error
β”‚        @ .\libuv.jl:100 [inlined]
β”‚      [2] unlink(p::String)
β”‚        @ Base.Filesystem .\file.jl:968
β”‚      [3] rm(path::String; force::Bool, recursive::Bool)
β”‚        @ Base.Filesystem .\file.jl:283
β”‚      [4] rm
β”‚        @ .\file.jl:274 [inlined]
β”‚      [5] mktemp(fn::Main.Test49Main_misc.var"#110#113", parent::String)
β”‚        @ Base.Filesystem .\file.jl:736
β”‚      [6] mktemp(fn::Function)
β”‚        @ Base.Filesystem .\file.jl:730
β”‚      [7] macro expansion
β”‚        @ C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\test\misc.jl:1065 [inlined]
β”‚      [8] macro expansion
β”‚        @ C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\stdlib\v1.9\Test\src\Test.jl:1360 [inlined]
β”‚      [9] top-level scope
β”‚        @ C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\test\misc.jl:1060
β”‚     [10] include
β”‚        @ .\Base.jl:427 [inlined]
β”‚     [11] macro expansion
β”‚        @ C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\test\testdefs.jl:24 [inlined]
β”‚     [12] macro expansion
β”‚        @ C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\stdlib\v1.9\Test\src\Test.jl:1360 [inlined]
β”‚     [13] macro expansion
β”‚        @ C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\test\testdefs.jl:23 [inlined]
β”‚     [14] macro expansion
β”‚        @ .\timing.jl:440 [inlined]
β”‚     [15] runtests(name::String, path::String, isolate::Bool; seed::UInt128)
β”‚        @ Main C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\test\testdefs.jl:21
β”‚     [16] invokelatest(::Any, ::Any, ::Vararg{Any}; kwargs::Base.Pairs{Symbol, UInt128, Tuple{Symbol}, NamedTuple{(:seed,), Tuple{UInt128}}})
β”‚        @ Base .\essentials.jl:798
β”‚     [17] (::Distributed.var"#110#112"{Distributed.CallMsg{:call_fetch}})()
β”‚        @ Distributed C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\stdlib\v1.9\Distributed\src\process_messages.jl:285
β”‚     [18] run_work_thunk(thunk::Distributed.var"#110#112"{Distributed.CallMsg{:call_fetch}}, print_error::Bool)
β”‚        @ Distributed C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\stdlib\v1.9\Distributed\src\process_messages.jl:70
β”‚     [19] macro expansion
β”‚        @ C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\stdlib\v1.9\Distributed\src\process_messages.jl:285 [inlined]
β”‚     [20] (::Distributed.var"#109#111"{Distributed.CallMsg{:call_fetch}, Distributed.MsgHeader, Sockets.TCPSocket})()
β”‚        @ Distributed .\task.jl:490
β”” @ Base.Filesystem file.jl:738
```
staticfloat pushed a commit that referenced this pull request Nov 1, 2022
This warning was un-ironically introduced by "Fix or suppress some noisy tests".
```
β”Œ Error: mktemp cleanup
β”‚   exception =
β”‚    IOError: unlink("C:\\Windows\\TEMP\\jl_A49B.tmp"): resource busy or locked (EBUSY)
β”‚    Stacktrace:
β”‚      [1] uv_error
β”‚        @ .\libuv.jl:100 [inlined]
β”‚      [2] unlink(p::String)
β”‚        @ Base.Filesystem .\file.jl:968
β”‚      [3] rm(path::String; force::Bool, recursive::Bool)
β”‚        @ Base.Filesystem .\file.jl:283
β”‚      [4] rm
β”‚        @ .\file.jl:274 [inlined]
β”‚      [5] mktemp(fn::Main.Test49Main_misc.var"#110#113", parent::String)
β”‚        @ Base.Filesystem .\file.jl:736
β”‚      [6] mktemp(fn::Function)
β”‚        @ Base.Filesystem .\file.jl:730
β”‚      [7] macro expansion
β”‚        @ C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\test\misc.jl:1065 [inlined]
β”‚      [8] macro expansion
β”‚        @ C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\stdlib\v1.9\Test\src\Test.jl:1360 [inlined]
β”‚      [9] top-level scope
β”‚        @ C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\test\misc.jl:1060
β”‚     [10] include
β”‚        @ .\Base.jl:427 [inlined]
β”‚     [11] macro expansion
β”‚        @ C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\test\testdefs.jl:24 [inlined]
β”‚     [12] macro expansion
β”‚        @ C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\stdlib\v1.9\Test\src\Test.jl:1360 [inlined]
β”‚     [13] macro expansion
β”‚        @ C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\test\testdefs.jl:23 [inlined]
β”‚     [14] macro expansion
β”‚        @ .\timing.jl:440 [inlined]
β”‚     [15] runtests(name::String, path::String, isolate::Bool; seed::UInt128)
β”‚        @ Main C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\test\testdefs.jl:21
β”‚     [16] invokelatest(::Any, ::Any, ::Vararg{Any}; kwargs::Base.Pairs{Symbol, UInt128, Tuple{Symbol}, NamedTuple{(:seed,), Tuple{UInt128}}})
β”‚        @ Base .\essentials.jl:798
β”‚     [17] (::Distributed.var"#110#112"{Distributed.CallMsg{:call_fetch}})()
β”‚        @ Distributed C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\stdlib\v1.9\Distributed\src\process_messages.jl:285
β”‚     [18] run_work_thunk(thunk::Distributed.var"#110#112"{Distributed.CallMsg{:call_fetch}}, print_error::Bool)
β”‚        @ Distributed C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\stdlib\v1.9\Distributed\src\process_messages.jl:70
β”‚     [19] macro expansion
β”‚        @ C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\stdlib\v1.9\Distributed\src\process_messages.jl:285 [inlined]
β”‚     [20] (::Distributed.var"#109#111"{Distributed.CallMsg{:call_fetch}, Distributed.MsgHeader, Sockets.TCPSocket})()
β”‚        @ Distributed .\task.jl:490
β”” @ Base.Filesystem file.jl:738
```

(cherry picked from commit d28a0dd)
staticfloat added a commit that referenced this pull request Nov 7, 2022
This warning was un-ironically introduced by "Fix or suppress some noisy tests".
```
β”Œ Error: mktemp cleanup
β”‚   exception =
β”‚    IOError: unlink("C:\\Windows\\TEMP\\jl_A49B.tmp"): resource busy or locked (EBUSY)
β”‚    Stacktrace:
β”‚      [1] uv_error
β”‚        @ .\libuv.jl:100 [inlined]
β”‚      [2] unlink(p::String)
β”‚        @ Base.Filesystem .\file.jl:968
β”‚      [3] rm(path::String; force::Bool, recursive::Bool)
β”‚        @ Base.Filesystem .\file.jl:283
β”‚      [4] rm
β”‚        @ .\file.jl:274 [inlined]
β”‚      [5] mktemp(fn::Main.Test49Main_misc.var"#110#113", parent::String)
β”‚        @ Base.Filesystem .\file.jl:736
β”‚      [6] mktemp(fn::Function)
β”‚        @ Base.Filesystem .\file.jl:730
β”‚      [7] macro expansion
β”‚        @ C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\test\misc.jl:1065 [inlined]
β”‚      [8] macro expansion
β”‚        @ C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\stdlib\v1.9\Test\src\Test.jl:1360 [inlined]
β”‚      [9] top-level scope
β”‚        @ C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\test\misc.jl:1060
β”‚     [10] include
β”‚        @ .\Base.jl:427 [inlined]
β”‚     [11] macro expansion
β”‚        @ C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\test\testdefs.jl:24 [inlined]
β”‚     [12] macro expansion
β”‚        @ C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\stdlib\v1.9\Test\src\Test.jl:1360 [inlined]
β”‚     [13] macro expansion
β”‚        @ C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\test\testdefs.jl:23 [inlined]
β”‚     [14] macro expansion
β”‚        @ .\timing.jl:440 [inlined]
β”‚     [15] runtests(name::String, path::String, isolate::Bool; seed::UInt128)
β”‚        @ Main C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\test\testdefs.jl:21
β”‚     [16] invokelatest(::Any, ::Any, ::Vararg{Any}; kwargs::Base.Pairs{Symbol, UInt128, Tuple{Symbol}, NamedTuple{(:seed,), Tuple{UInt128}}})
β”‚        @ Base .\essentials.jl:798
β”‚     [17] (::Distributed.var"#110#112"{Distributed.CallMsg{:call_fetch}})()
β”‚        @ Distributed C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\stdlib\v1.9\Distributed\src\process_messages.jl:285
β”‚     [18] run_work_thunk(thunk::Distributed.var"#110#112"{Distributed.CallMsg{:call_fetch}}, print_error::Bool)
β”‚        @ Distributed C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\stdlib\v1.9\Distributed\src\process_messages.jl:70
β”‚     [19] macro expansion
β”‚        @ C:\buildbot\worker-tabularasa\tester_win64\build\share\julia\stdlib\v1.9\Distributed\src\process_messages.jl:285 [inlined]
β”‚     [20] (::Distributed.var"#109#111"{Distributed.CallMsg{:call_fetch}, Distributed.MsgHeader, Sockets.TCPSocket})()
β”‚        @ Distributed .\task.jl:490
β”” @ Base.Filesystem file.jl:738
```

(cherry picked from commit d28a0dd)

Co-authored-by: Jameson Nash <[email protected]>
@staticfloat staticfloat mentioned this pull request Nov 7, 2022
37 tasks
vchuravy pushed a commit to JuliaPackaging/LazyArtifacts.jl that referenced this pull request Oct 2, 2023
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.

5 participants