Fix breaking tests #46058
Fix breaking tests #46058
Conversation
|
#46055 was merged before you opened this PR. |
|
I see, then I'll try to extract the misc fix... |
| @@ -969,7 +969,7 @@ include("testenv.jl") | |||
|
|
|||
|
|
|||
| let flags = Cmd(filter(a->!occursin("depwarn", a), collect(test_exeflags))) | |||
| local cmd = `$test_exename $flags --depwarn=yes deprecation_exec.jl` | |||
| local cmd = `$test_exename $flags --depwarn=yes $(joinpath(@__DIR__, "deprecation_exec.jl"))` | |||
There was a problem hiding this comment.
What problem does this solve? I'm not sure I've seen any error related to this.
There was a problem hiding this comment.
Error in testset misc:
| Error During Test at /Users/administrator/.julia/scratchspaces/a66863c6-20e8-4ff4-8a62-49f30b1f605e/agent-cache/default-macmini-aarch64-1.0/build/default-macmini-aarch64-1-0/julialang/julia-master/julia-6a83702afc/share/julia/test/testdefs.jl:21
| Got exception outside of a `@test`
| LoadError: Deprecation test failed
There was a problem hiding this comment.
Do you have a link to a failed job? That snippet doesn't say much. Also, I find it strange that that line has been last modified 2 years ago: https://github.com/Rratic/julia/blame/d957f310028b564d223097368f7cfd2e609bd638/test/misc.jl#L972. How come this is a problem only now?
There was a problem hiding this comment.
https://buildkite.com/julialang/julia-master/builds/13900#0181fd37-7de5-4283-a971-798da8bbb6c9
Not sure where the error exactly is
There was a problem hiding this comment.
Yeah, not clear. The change is probably ok in any case, but I'm not convinced it solves anything, since we don't know what the problem is (and again, this line hasn't been a problem for the past two years).
There was a problem hiding this comment.
I don't think this change is necessary. The deprecation_exec tests are passing just fine on master. So I don't think there is a problem with the test itself.
I do think it will be helpful to print some more output when the test fails, which is what #46059 dos.
|
Given #46039 (comment), I believe the title of this PR is at very least misleading as it doesn't fix anything. I think the change itself is OK, but also unnecessary, not sure it's worth it. |
| @@ -969,7 +969,7 @@ include("testenv.jl") | |||
|
|
|||
|
|
|||
| let flags = Cmd(filter(a->!occursin("depwarn", a), collect(test_exeflags))) | |||
| local cmd = `$test_exename $flags --depwarn=yes deprecation_exec.jl` | |||
| local cmd = `$test_exename $flags --depwarn=yes $(joinpath(@__DIR__, "deprecation_exec.jl"))` | |||
There was a problem hiding this comment.
I don't think this change is necessary. The deprecation_exec tests are passing just fine on master. So I don't think there is a problem with the test itself.
I do think it will be helpful to print some more output when the test fails, which is what #46059 dos.
These failed tests are breaking every pr check.
Hope this fixes...