Skip to content

Fix @test_trixi_include_base to support locally-defined kwarg values #32

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.

Sign up for GitHub

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

Merged
ranocha merged 12 commits into from
Jun 19, 2026
Merged

Fix @test_trixi_include_base to support locally-defined kwarg values #32

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
5ee263a
fix locally-defined overwrites in at-test_trixi_include
JoshuaLampert Jun 9, 2026
17884fb
fix for julia v1.10
JoshuaLampert Jun 9, 2026
bdd9c2e
format
JoshuaLampert Jun 9, 2026
ed25f76
remove unclear reference
JoshuaLampert Jun 9, 2026
ec15b71
add more tests
JoshuaLampert Jun 10, 2026
7b9af4f
fix
JoshuaLampert Jun 10, 2026
03c8833
remove test_trixi_include to avoid overwriting f
JoshuaLampert Jun 10, 2026
37714cf
add more tests for locally defined overrides
JoshuaLampert Jun 10, 2026
42709c6
move definition of f outside example
JoshuaLampert Jun 10, 2026
696388b
add tests for NamedTuple and function keyword argument
JoshuaLampert Jun 13, 2026
281695a
remove julia version gate
JoshuaLampert Jun 13, 2026
4d0c5c4
test more cases
ranocha Jun 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 33 additions & 4 deletions src/macros.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,18 +74,47 @@ macro test_trixi_include_base(elixir, args...)
local atol = get_kwarg(args, :atol, atol_default)
local rtol = get_kwarg(args, :rtol, rtol_default)

local kwargs = Pair{Symbol, Any}[]
# Build escaped kwarg expressions. For bare-symbol values there are three cases:
# 1. Symbol is also a key in this kwarg list (e.g. `seed=6, x=seed`): always pass
# as Symbol so trixi_include resolves it inside the elixir after the other
# override (seed=6) has been applied.
# 2. Locally-defined values defined in the testset body:
# @isdefined returns true (same world age), so the actual value is passed.
# 3. Elixir-internal variable references (e.g. bare `x=seed` with no seed= key):
# on Julia >= 1.12, @isdefined returns false because bindings set inside
# Base.include have a newer world age; on older Julia the value is visible and
# also correct (same as the elixir default).
# For non-Symbol expressions (closures, calls, literals), always evaluate at call site.
local kwarg_keys = Set(arg.args[1]
for arg in args
if arg.head == :(=) &&
!(arg.args[1] in (:additional_ignore_content, :l2, :linf,
:RealT_for_test_tolerances, :atol, :rtol)))
local kwarg_exprs = Expr[]
for arg in args
if (arg.head == :(=) &&
!(arg.args[1] in (:additional_ignore_content, :l2, :linf,
:RealT_for_test_tolerances, :atol, :rtol)))
push!(kwargs, Pair(arg.args...))
key = arg.args[1]
val = arg.args[2]
if val isa Symbol && val in kwarg_keys
# Case 1: chained override — pass as Symbol for elixir-internal resolution
push!(kwarg_exprs, Expr(:kw, key, QuoteNode(val)))
elseif val isa Symbol
Comment thread
ranocha marked this conversation as resolved.
# Cases 2 & 3: use @isdefined to capture locally-defined values while
# falling back to Symbol for elixir-internal references
push!(kwarg_exprs,
Expr(:kw, key,
esc(:((@isdefined $val) ? $val : $(QuoteNode(val))))))
Comment thread
ranocha marked this conversation as resolved.

@vchuravy vchuravy Jun 13, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Isn't your comment above saying that this won't work for 1.12

@JoshuaLampert JoshuaLampert Jun 13, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Not sure what exactly you mean, but $val does not work for julia v1.12 (but it works for <v1.12), but $(QuoteNode(val)) does work on julia v1.12. That is exactly why we have the @isdefined $val (true for julia <v1.12, false for julia v1.12).

@vchuravy vchuravy Jun 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

  1. Elixir-internal variable references (e.g. bare x=seed with no seed= key):
    on Julia >= 1.12, @isdefined returns false because bindings set inside
    Base.include have a newer world age; on older Julia the value is visible and
    also correct (same as the elixir default).

@vchuravy vchuravy Jun 13, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Also please /simplify the comments. Claude and co leave very verbose and often wrong comments.

@JoshuaLampert JoshuaLampert Jun 13, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I am still not sure why the comment should contradict what is implemented. I find Claude's comment (I already shortened it slightly) helpful, but if you have a concrete suggestion, please make it.
If you want to know what Claude says to your question, here is its answer (sorry for copy-pasting LLM answers; I usually don't do that, but think here it's the easiest):

The reviewer is reading the comment for Case 3 — "on Julia ≥ 1.12, @isdefined returns false because bindings set inside Base.include have a newer world age" — and then seeing that line 108 uses the exact same @isdefined check for Cases 2 & 3 combined. They're asking: if @isdefined returns false on Julia ≥ 1.12, doesn't that break Case 2 (locally-defined values) as well?

The answer is no, and the reason is the distinction the comment is trying to make: @isdefined only returns false for bindings whose world age is newer than the current scope. Locally-defined values in the testset body (Case 2, like local_x = 6) are created at the same world age as the testset closure — so @isdefined local_x returns true on all Julia versions. Only bindings created inside Base.include (when trixi_include runs the elixir) are at a newer world age and therefore invisible via @isdefined on Julia ≥ 1.12.

The comment is correct but doesn't make this distinction explicit enough. It reads as if @isdefined is generally unreliable on Julia ≥ 1.12, when it's only unreliable for elixir-internal bindings. A clearer phrasing would be something like:

  1. Elixir-internal variable references (e.g. bare x=seed with no seed= key):
    on Julia >= 1.12, @isdefined returns false because these bindings are created
    inside Base.include at a newer world age than the testset body; on older Julia
    the value is visible and also correct (same as the elixir default).
    Note: locally-defined values (case 2) are at the same world age as the testset,
    so @isdefined correctly returns true for them on all Julia versions.

Does this answer your question? If yes, should we adjust the comment for clarification, should I reduce the comment further (as you suggested), or is it fine? If not, could you please explain more your concerns, ideally by making suggestions to improve the comment or asking for concrete examples I can test?

else
push!(kwarg_exprs, Expr(:kw, key, esc(val)))
end
end
end

# if `maxiters` is set in tests, it is usually set to a small number to
# run only a few steps - ignore possible warnings coming from that
if any(==(:maxiters) ∘ first, kwargs)
if any(e -> e.args[1] == :maxiters, kwarg_exprs)
args = append_to_kwargs(args, :additional_ignore_content,
[
r"┌ Warning: Verbosity toggle: max_iters \n│ Interrupted\. Larger maxiters is needed\..*\n└ @ SciMLBase .+\n",
Expand All @@ -99,7 +128,7 @@ macro test_trixi_include_base(elixir, args...)
mpi_isroot() && println($(esc(elixir)))

# evaluate examples in the scope of the module they're called from
@trixi_test_nowarn trixi_include(@__MODULE__, $(esc(elixir)); $kwargs...) $additional_ignore_content
@trixi_test_nowarn trixi_include(@__MODULE__, $(esc(elixir)); $(kwarg_exprs...)) $additional_ignore_content

# if present, compare l2 and linf errors against reference values
if !isnothing($l2) || !isnothing($linf)
Expand Down
253 changes: 158 additions & 95 deletions test/test_test_trixi_include.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,45 +20,25 @@ end

# just include
@test_trixi_include_base(path)
if VERSION >= v"1.12"
mod = @__MODULE__
@test @invokelatest isdefined(mod, :x)
@test (@invokelatest mod.x) == 4
else
@test @isdefined x
@test x == 4
end
mod = @__MODULE__
@test @invokelatest isdefined(mod, :x)
@test (@invokelatest mod.x) == 4

@test_trixi_include(path)
if VERSION >= v"1.12"
mod = @__MODULE__
@test @invokelatest isdefined(mod, :x)
@test (@invokelatest mod.x) == 4
else
@test @isdefined x
@test x == 4
end
mod = @__MODULE__
@test @invokelatest isdefined(mod, :x)
@test (@invokelatest mod.x) == 4

# include and overwrite included variable by a constant
@test_trixi_include_base(path, x=9)
if VERSION >= v"1.12"
mod = @__MODULE__
@test @invokelatest isdefined(mod, :x)
@test (@invokelatest mod.x) == 9
else
@test @isdefined x
@test x == 9
end
mod = @__MODULE__
@test @invokelatest isdefined(mod, :x)
@test (@invokelatest mod.x) == 9

@test_trixi_include(path, x=9)
if VERSION >= v"1.12"
mod = @__MODULE__
@test @invokelatest isdefined(mod, :x)
@test (@invokelatest mod.x) == 9
else
@test @isdefined x
@test x == 9
end
mod = @__MODULE__
@test @invokelatest isdefined(mod, :x)
@test (@invokelatest mod.x) == 9
end
end

Expand All @@ -75,66 +55,159 @@ end
# overwrite included variable by a (global) variable
global override = 5
@test_trixi_include_base(path, x=override)
if VERSION >= v"1.12"
mod = @__MODULE__
@test @invokelatest isdefined(mod, :x)
@test (@invokelatest mod.x) == 5
else
@test @isdefined x
@test x == 5
end
mod = @__MODULE__
@test @invokelatest isdefined(mod, :x)
@test (@invokelatest mod.x) == 5

@test_trixi_include(path, x=override)
if VERSION >= v"1.12"
mod = @__MODULE__
@test @invokelatest isdefined(mod, :x)
@test (@invokelatest mod.x) == 5
else
@test @isdefined x
@test x == 5
end
mod = @__MODULE__
@test @invokelatest isdefined(mod, :x)
@test (@invokelatest mod.x) == 5

# overwrite included variable by another included variable
@test_trixi_include_base(path, x=seed)
if VERSION >= v"1.12"
mod = @__MODULE__
@test @invokelatest isdefined(mod, :x)
@test (@invokelatest mod.x) == 42
else
@test @isdefined x
@test x == 42
end
mod = @__MODULE__
@test @invokelatest isdefined(mod, :x)
@test (@invokelatest mod.x) == 42

@test_trixi_include(path, x=seed)
if VERSION >= v"1.12"
mod = @__MODULE__
@test @invokelatest isdefined(mod, :x)
@test (@invokelatest mod.x) == 42
else
@test @isdefined x
@test x == 42
end
mod = @__MODULE__
@test @invokelatest isdefined(mod, :x)
@test (@invokelatest mod.x) == 42

# overwrite included variable by supplied variable
@test_trixi_include_base(path, seed=6, x=seed)
if VERSION >= v"1.12"
mod = @__MODULE__
@test @invokelatest isdefined(mod, :x)
@test (@invokelatest mod.x) == 6
else
@test @isdefined x
@test x == 6
end
mod = @__MODULE__
@test @invokelatest isdefined(mod, :x)
@test (@invokelatest mod.x) == 6

@test_trixi_include(path, seed=6, x=seed)
if VERSION >= v"1.12"
mod = @__MODULE__
@test @invokelatest isdefined(mod, :x)
@test (@invokelatest mod.x) == 6
else
@test @isdefined x
@test x == 6
mod = @__MODULE__
@test @invokelatest isdefined(mod, :x)
@test (@invokelatest mod.x) == 6
end
end

@trixi_testset "normal override, all assignment forms" begin
global f(; x = 0) = x
example = """
x = 1
x_kw_pos = f(x = 1)
x_kw_semi = f(; x = 1)
y = (; x = 1)
function g(; x = 1)
return x
end
y_g = g()
function h(x = 1)
return x
end
y_h = h()
y_let = 0
y_let_global = 0
let x = 1
y_let = x
global y_let_global = x
end
"""
Comment thread
ranocha marked this conversation as resolved.

mktemp() do path, io
write(io, example)
close(io)

@test_trixi_include_base(path, x=6)
mod = @__MODULE__
@test (@invokelatest mod.x) == 6
@test (@invokelatest mod.x_kw_pos) == 6
@test (@invokelatest mod.x_kw_semi) == 6
@test (@invokelatest mod.y).x == 6
@test (@invokelatest mod.y_g) == 6
@test (@invokelatest mod.y_h) == 6
@test (@invokelatest mod.y_let) == 0 # let block introduces a local scope
@test (@invokelatest mod.y_let_global) == 6
end
end

@trixi_testset "chained override, all assignment forms" begin
global f(; x = 0) = x
example = """
seed = 42
x = 1
x_kw_pos = f(x = 1)
x_kw_semi = f(; x = 1)
y = (; x = 1)
function g(; x = 1)
return x
end
y_g = g()
function h(x = 1)
return x
end
y_h = h()
y_let = 0
y_let_global = 0
let x = 1
y_let = x
global y_let_global = x
end
"""

mktemp() do path, io
write(io, example)
close(io)

@test_trixi_include_base(path, seed=6, x=seed)
mod = @__MODULE__
@test (@invokelatest mod.x) == 6
@test (@invokelatest mod.x_kw_pos) == 6
@test (@invokelatest mod.x_kw_semi) == 6
@test (@invokelatest mod.y).x == 6
@test (@invokelatest mod.y_g) == 6
@test (@invokelatest mod.y_h) == 6
@test (@invokelatest mod.y_let) == 0 # let block introduces a local scope
@test (@invokelatest mod.y_let_global) == 6
end
end

@trixi_testset "locally defined override, all assignment forms" begin
global f(; x = 0) = x
example = """
x = 1
x_kw_pos = f(x = 1)
x_kw_semi = f(; x = 1)
y = (; x = 1)
function g(; x = 1)
return x
end
y_g = g()
function h(x = 1)
return x
end
y_h = h()
y_let = 0
y_let_global = 0
let x = 1
y_let = x
global y_let_global = x
end
"""

Comment thread
ranocha marked this conversation as resolved.
mktemp() do path, io
write(io, example)
close(io)

# overwrite included variable by a locally defined value (not a module global)
local_x = 6
@test_trixi_include_base(path, x=local_x)
mod = @__MODULE__
@test (@invokelatest mod.x) == 6
@test (@invokelatest mod.x_kw_pos) == 6
@test (@invokelatest mod.x_kw_semi) == 6
@test (@invokelatest mod.y).x == 6
@test (@invokelatest mod.y_g) == 6
@test (@invokelatest mod.y_h) == 6
@test (@invokelatest mod.y_let) == 0 # let block introduces a local scope
@test (@invokelatest mod.y_let_global) == 6
end
end

Expand Down Expand Up @@ -236,24 +309,14 @@ end
close(io)

@test_trixi_include_base(path, RealT=Float32)
if VERSION >= v"1.12"
mod = @__MODULE__
@test @invokelatest isdefined(mod, :RealT)
@test (@invokelatest mod.RealT) == Float32
else
@test @isdefined RealT
@test RealT == Float32
end
mod = @__MODULE__
@test @invokelatest isdefined(mod, :RealT)
@test (@invokelatest mod.RealT) == Float32

@test_trixi_include(path, RealT=Float32)
if VERSION >= v"1.12"
mod = @__MODULE__
@test @invokelatest isdefined(mod, :RealT)
@test (@invokelatest mod.RealT) == Float32
else
@test @isdefined RealT
@test RealT == Float32
end
mod = @__MODULE__
@test @invokelatest isdefined(mod, :RealT)
@test (@invokelatest mod.RealT) == Float32
end
end
end
Loading