Skip to content

runtests: prefix with the testsuite and worker number#60372

Open
IanButterworth wants to merge 5 commits intoJuliaLang:masterfrom
IanButterworth:ib/runtests_prefix
Open

runtests: prefix with the testsuite and worker number#60372
IanButterworth wants to merge 5 commits intoJuliaLang:masterfrom
IanButterworth:ib/runtests_prefix

Conversation

@IanButterworth
Copy link
Member

@IanButterworth IanButterworth commented Dec 12, 2025

Depends on JuliaLang/Distributed.jl#157

Shows the test suite name as the output prefix to make it easier to figure out which test it came from

https://buildkite.com/julialang/julia-master/builds/52736#019b13f7-46fa-4266-8d65-b785869b9c06/954-1074

Screenshot 2025-12-12 at 2 34 08 PM

@gbaraldi
Copy link
Member

What am I looking for as a difference

@IanButterworth
Copy link
Member Author

It's not currently working

From worker 4: Precompiling packages...
From worker 4:     262.0 ms  ✓ Foo51989
From worker 4:   1 dependency successfully precompiled in 0 seconds

Should look like

loading (4): Precompiling packages...
loading (4):     262.0 ms  ✓ Foo51989
loading (4):  1 dependency successfully precompiled in 0 seconds

@IanButterworth
Copy link
Member Author

@IanButterworth
Copy link
Member Author

I think it's looking good, but not sure if the changes to Distributed are the best way to do it i.e. via a global hook..
JuliaLang/Distributed.jl#157

@IanButterworth IanButterworth marked this pull request as ready for review December 18, 2025 03:24
@IanButterworth
Copy link
Member Author

@JamesWrigley would you mind reviewing this and JuliaLang/Distributed.jl#157

@JamesWrigley
Copy link
Member

Sure, will try to give it a look over the weekend 👍

@IanButterworth
Copy link
Member Author

@JamesWrigley did you have any feedback on this? thanks

@JamesWrigley
Copy link
Member

My apologies, I forgot 🙈 Will take a look tomorrow.

Copy link
Member

@JamesWrigley JamesWrigley left a comment

Choose a reason for hiding this comment

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

My apologies, I forgot 🙈 Will take a look tomorrow.

In my defense I had food poisoning last week 🤣

Comment on lines +39 to +43
redirect_stdout(pipe) do
redirect_stderr(pipe) do
f()
end
end
Copy link
Member

Choose a reason for hiding this comment

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

Nitpicking:

Suggested change
redirect_stdout(pipe) do
redirect_stderr(pipe) do
f()
end
end
redirect_stdio(; stdout=pipe, stderr=pipe) do
f()
end

try
while isopen(pipe) || bytesavailable(pipe) > 0
line = readline(pipe; keep=true)
isempty(line) && break
Copy link
Member

Choose a reason for hiding this comment

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

What is this check for?

end
finally
close(pipe.in)
wait(reader_task)
Copy link
Member

Choose a reason for hiding this comment

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

Probably happens automatically through the finalizer but just to be safe:

Suggested change
wait(reader_task)
wait(reader_task)
close(pipe)

test_name = wrkr_id === nothing ? nothing : get(worker_current_test, wrkr_id, nothing)
@lock print_lock begin
if test_name !== nothing
printstyled(" ", test_name, " (", ident, "): ", color=:light_black)
Copy link
Member

Choose a reason for hiding this comment

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

Ah I think I now see why you wanted the callback to control printing 😅 Could we maybe use StyledStrings for this? Would that handle printing stuff correctly to an IO that may or may not support colors?

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.

3 participants