From 768a40d0fc0968fe8ac64b97d3f554413ce6e69c Mon Sep 17 00:00:00 2001
From: David Anthoff <anthoff@berkeley.edu>
Date: Thu, 13 Aug 2026 16:09:23 -0700
Subject: [PATCH 1/2] Carry `option_skip`, and make test item ids stable
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Two changes to the test item discovery layer.

`TestItemDetail` gains `option_skip::Union{Bool,String}`: a literal
`true`/`false`, or the source text of an expression that the test process
evaluates just before the item would run. The expression text is sliced out of
the file content the same way `code` is.

Test item ids change from the positional `"$uri:$i"` to
`"<relpath>::<label>"`, where relpath is the file's path relative to its
package root, normalized to `/` separators so Windows and Linux agree on the
same id. Positional ids renumber every item below an insertion, which is
invisible today but breaks the moment ids are persisted (JUnit output, flake
history, failures-first ordering) — and readable ids are also nicer in
`--filter` expressions and rerun-by-id flows.

A label used more than once in one file now gets both a `#N` suffix on
*every* occurrence — so ids stay unique, each item stays individually
addressable, and the error state is visible in the id itself — and a
definition error through the existing `testerrors` channel, so the editor
shows a squiggle and the extension can render an error node. Duplicate
`@testmodule`/`@testsnippet` names get the same definition error.

This is a breaking change to the id format. Downstream consumers needing a
lockstep update are listed in the PR body.

Part of stream S1 (Discovery) of the ReTestItems feature-delta plan
(A.2, A.10 items 2 and 5).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 src/layer_testitems.jl | 96 ++++++++++++++++++++++++++++++++++++++----
 src/types.jl           |  5 ++-
 test/test_testitems.jl | 95 +++++++++++++++++++++++++++++++++++++++--
 3 files changed, 183 insertions(+), 13 deletions(-)

diff --git a/src/layer_testitems.jl b/src/layer_testitems.jl
index 535b1bc7..c749c55e 100644
--- a/src/layer_testitems.jl
+++ b/src/layer_testitems.jl
@@ -57,6 +57,36 @@ Salsa.@derived function derived_testitems_selected(rt, uri)
     return path_selected(parse_path_filter!(discard, toml_content), relpath)
 end
 
+"""
+    testitem_relative_path(package_uri, uri) -> String
+
+The path of `uri` relative to its package root, with `/` separators. This is the
+first half of a test item id, so it must be stable across machines: relative so a
+dev checkout and a CI runner agree, `/`-separated so Windows and Linux do. Falls
+back to the full URI when there is no filesystem path to work with.
+"""
+function testitem_relative_path(package_uri::Union{URI,Nothing}, uri::URI)
+    if package_uri !== nothing
+        package_path = uri2filepath(package_uri)
+        file_path = uri2filepath(uri)
+
+        if package_path !== nothing && file_path !== nothing
+            relpath = config_relative_path(package_path, file_path)
+            relpath === nothing || return relpath
+        end
+    end
+
+    return string(uri)
+end
+
+function _label_counts(labels)
+    counts = Dict{String,Int}()
+    for label in labels
+        counts[label] = get(counts, label, 0) + 1
+    end
+    return counts
+end
+
 Salsa.@derived function derived_testitems(rt, uri)
     @debug "derived_testitems" uri=uri
 
@@ -118,17 +148,71 @@ Salsa.@derived function derived_testitems(rt, uri)
         )
     end
 
+    all_testerrors = TestErrorDetail[
+        TestErrorDetail(
+            uri,
+            "$uri:error$i",
+            string(te.name),
+            te.message,
+            te.range
+        ) for (i,te) in enumerate(testerrors)
+    ]
+
+    # Ids are `<path relative to the package>::<label>`, so inserting a test item
+    # above another one no longer renumbers it. A label used more than once in one
+    # file is a definition error, but the run must still degrade rather than break,
+    # so *every* occurrence gets a `#N` suffix — that keeps ids unique, keeps each
+    # item individually addressable, and makes the error state visible in the id.
+    relpath = testitem_relative_path(package_uri, uri)
+
+    item_labels = String[ti.name for ti in testitems]
+    item_counts = _label_counts(item_labels)
+    seen_items = Dict{String,Int}()
+    item_ids = Vector{String}(undef, length(testitems))
+
+    for (i, label) in enumerate(item_labels)
+        if item_counts[label] > 1
+            occurrence = seen_items[label] = get(seen_items, label, 0) + 1
+            item_ids[i] = "$relpath::$label#$occurrence"
+
+            push!(all_testerrors, TestErrorDetail(
+                uri,
+                "$uri:error$(length(all_testerrors) + 1)",
+                label,
+                "The test item name \"$label\" is used more than once in this file. Test item names must be unique within a file.",
+                testitems[i].range
+            ))
+        else
+            item_ids[i] = "$relpath::$label"
+        end
+    end
+
+    setup_counts = _label_counts(String[string(ts.name) for ts in testsetups])
+
+    for ts in testsetups
+        if setup_counts[string(ts.name)] > 1
+            push!(all_testerrors, TestErrorDetail(
+                uri,
+                "$uri:error$(length(all_testerrors) + 1)",
+                string(ts.name),
+                "The test setup name `$(ts.name)` is used more than once in this file. Test setup names must be unique within a file.",
+                ts.range
+            ))
+        end
+    end
+
     return TestDetails(
         [TestItemDetail(
             uri,
-            "$uri:$i",
+            item_ids[i],
             ti.name,
             text_file.content.content[ti.code_range],
             ti.range,
             ti.code_range,
             ti.option_default_imports,
             ti.option_tags,
-            ti.option_setup
+            ti.option_setup,
+            ti.option_skip isa Bool ? ti.option_skip : text_file.content.content[ti.option_skip]
             ) for (i,ti) in enumerate(testitems)],
         [TestSetupDetail(
             uri,
@@ -138,13 +222,7 @@ Salsa.@derived function derived_testitems(rt, uri)
             i.range,
             i.code_range
             ) for i in testsetups],
-        [TestErrorDetail(
-            uri,
-            "$uri:error$i",
-            string(te.name),
-            te.message,
-            te.range
-            ) for (i,te) in enumerate(testerrors)]
+        all_testerrors
     )
 end
 
diff --git a/src/types.jl b/src/types.jl
index 1857efb3..34c47430 100644
--- a/src/types.jl
+++ b/src/types.jl
@@ -19,6 +19,8 @@ Details of a test item.
 - `option_default_imports`::Bool
 - option_tags::Vector{Symbol}
 - option_setup::Vector{Symbol}
+- `option_skip`::Union{Bool,String} — a literal `true`/`false`, or the source text of an
+  expression that the test process evaluates just before the test item would run.
 """
 struct TestItemDetail
     uri::URI
@@ -30,8 +32,9 @@ struct TestItemDetail
     option_default_imports::Bool
     option_tags::Vector{Symbol}
     option_setup::Vector{Symbol}
+    option_skip::Union{Bool,String}
 end
-_key(x::TestItemDetail) = (x.uri, x.id, x.name, x.code, _range_key(x.range), _range_key(x.code_range), x.option_default_imports, x.option_tags, x.option_setup)
+_key(x::TestItemDetail) = (x.uri, x.id, x.name, x.code, _range_key(x.range), _range_key(x.code_range), x.option_default_imports, x.option_tags, x.option_setup, x.option_skip)
 Base.:(==)(a::TestItemDetail, b::TestItemDetail) = _key(a) == _key(b)
 Base.isequal(a::TestItemDetail, b::TestItemDetail) = isequal(_key(a), _key(b))
 Base.hash(x::TestItemDetail, h::UInt) = hash(_key(x), hash(TestItemDetail, h))
diff --git a/test/test_testitems.jl b/test/test_testitems.jl
index 327bc011..f27354c5 100644
--- a/test/test_testitems.jl
+++ b/test/test_testitems.jl
@@ -658,8 +658,8 @@ end
     @test !isequal(te_a, te_b)
     @test hash(te_a) != hash(te_b)
 
-    ti_a = TestItemDetail(u, "id", "n", "code", 24:23, 24:23, true, Symbol[], Symbol[])
-    ti_b = TestItemDetail(u, "id", "n", "code", 23:22, 23:22, true, Symbol[], Symbol[])
+    ti_a = TestItemDetail(u, "id", "n", "code", 24:23, 24:23, true, Symbol[], Symbol[], false)
+    ti_b = TestItemDetail(u, "id", "n", "code", 23:22, 23:22, true, Symbol[], Symbol[], false)
     @test ti_a != ti_b
     @test !isequal(ti_a, ti_b)
     @test hash(ti_a) != hash(ti_b)
@@ -672,10 +672,99 @@ end
 
     # Identical values still compare equal (backdating must still work).
     @test te_b == TestErrorDetail(u, "id", "n", "msg", 23:22)
-    @test isequal(ti_b, TestItemDetail(u, "id", "n", "code", 23:22, 23:22, true, Symbol[], Symbol[]))
+    @test isequal(ti_b, TestItemDetail(u, "id", "n", "code", 23:22, 23:22, true, Symbol[], Symbol[], false))
     @test hash(ts_b) == hash(TestSetupDetail(u, :n, :k, "code", 23:22, 23:22))
 end
 
+@testsnippet TestItemPackage begin
+    using JuliaWorkspaces: JuliaWorkspace, add_file!, TextFile, SourceText, get_test_items
+    using JuliaWorkspaces.URIs2: @uri_str, URI
+
+    # A minimal in-memory package, so that test items in `test/bar.jl` resolve to a
+    # package root and therefore get an id relative to it.
+    function workspace_with(content)
+        jw = JuliaWorkspace()
+
+        add_file!(jw, TextFile(uri"file:///home/foo/Project.toml", SourceText("name = \"Foo\"\nuuid = \"12345678-1234-1234-1234-123456789012\"\nversion = \"0.1.0\"\n", "toml")))
+        add_file!(jw, TextFile(uri"file:///home/foo/src/Foo.jl", SourceText("module Foo\nend\n", "julia")))
+        add_file!(jw, TextFile(uri"file:///home/foo/test/bar.jl", SourceText(content, "julia")))
+
+        return jw, uri"file:///home/foo/test/bar.jl"
+    end
+end
+
+@testitem "skip defaults to false" setup=[TestItemPackage] begin
+    jw, uri = workspace_with("""@testitem "foo" begin end""")
+
+    test_results = get_test_items(jw, uri)
+
+    @test length(test_results.testitems) == 1
+    @test test_results.testitems[1].option_skip === false
+end
+
+@testitem "skip literal is carried through" setup=[TestItemPackage] begin
+    jw, uri = workspace_with("""@testitem "foo" skip=true begin end\n@testitem "bar" skip=false begin end""")
+
+    test_results = get_test_items(jw, uri)
+
+    @test length(test_results.testerrors) == 0
+    @test test_results.testitems[1].option_skip === true
+    @test test_results.testitems[2].option_skip === false
+end
+
+@testitem "skip expression is carried through as source text" setup=[TestItemPackage] begin
+    jw, uri = workspace_with("""@testitem "foo" skip=(VERSION < v"1.11") begin end""")
+
+    test_results = get_test_items(jw, uri)
+
+    @test length(test_results.testerrors) == 0
+    # Parentheses are trivia to JuliaSyntax, so only the expression itself is sliced.
+    @test test_results.testitems[1].option_skip == "VERSION < v\"1.11\""
+end
+
+@testitem "test item ids are package relative and label based" setup=[TestItemPackage] begin
+    jw, uri = workspace_with("""@testitem "foo" begin end\n@testitem "bar" begin end""")
+
+    test_results = get_test_items(jw, uri)
+
+    @test [ti.id for ti in test_results.testitems] == ["test/bar.jl::foo", "test/bar.jl::bar"]
+end
+
+@testitem "test item ids are invariant under inserting an item above" setup=[TestItemPackage] begin
+    jw1, uri = workspace_with("""@testitem "foo" begin end""")
+    jw2, _ = workspace_with("""@testitem "inserted" begin end\n@testitem "foo" begin end""")
+
+    id1 = only(get_test_items(jw1, uri).testitems).id
+    id2 = get_test_items(jw2, uri).testitems[2].id
+
+    @test id1 == id2 == "test/bar.jl::foo"
+end
+
+@testitem "duplicate test item labels get numbered ids and a definition error" setup=[TestItemPackage] begin
+    jw, uri = workspace_with("""@testitem "foo" begin end\n@testitem "foo" begin end\n@testitem "bar" begin end""")
+
+    test_results = get_test_items(jw, uri)
+
+    # Every occurrence is suffixed, not just the second one, so the error state is
+    # visible in the id itself and each item stays individually addressable.
+    @test [ti.id for ti in test_results.testitems] == ["test/bar.jl::foo#1", "test/bar.jl::foo#2", "test/bar.jl::bar"]
+
+    @test length(test_results.testerrors) == 2
+    @test all(te -> te.name == "foo", test_results.testerrors)
+    @test all(te -> occursin("used more than once", te.message), test_results.testerrors)
+    @test allunique(te.id for te in test_results.testerrors)
+end
+
+@testitem "duplicate test setup names produce a definition error" setup=[TestItemPackage] begin
+    jw, uri = workspace_with("""@testmodule Foo begin end\n@testsnippet Foo begin end""")
+
+    test_results = get_test_items(jw, uri)
+
+    @test length(test_results.testsetups) == 2
+    @test length(test_results.testerrors) == 2
+    @test all(te -> te.name == "Foo", test_results.testerrors)
+end
+
 @testitem "get_test_items on untitled (non-file) URI" begin
     using JuliaWorkspaces: JuliaWorkspace, add_file!, TextFile, SourceText, get_test_items
     using JuliaWorkspaces.URIs2: @uri_str

From b4898b35e3569776a150be19c2b8f562520c3f7c Mon Sep 17 00:00:00 2001
From: David Anthoff <anthoff@berkeley.edu>
Date: Thu, 13 Aug 2026 16:53:45 -0700
Subject: [PATCH 2/2] Require TestItemDetection 1.2

`derived_testitems` reads `option_skip`, which TestItemDetection only
returns from v1.2.0 onwards.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 Project.toml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Project.toml b/Project.toml
index 4e61c3db..d13e4c03 100644
--- a/Project.toml
+++ b/Project.toml
@@ -31,7 +31,7 @@ TestItemRunner = "f8b46487-2199-4994-9208-9a1283c18c0a"
 Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"
 
 [compat]
-TestItemDetection = "1.1"
+TestItemDetection = "1.2"
 JuliaSyntax = "0.4, 1"
 PrecompileTools = "1.2"
 SHA = "<0.0.1, 0.7, 1"