Provide unique IDs for all node info objects

[Vampire]

Summary by CodeRabbit

• New Features
  • Test specifications, features, and iterations now receive stable, globally unique identifiers for improved tracking and reporting.
  • Iterations are identified relative to their parent feature (e.g., feature[id][index]) to distinguish parameterized runs.
  • Test nodes expose readable IDs for tooling and reporting, enabling clearer traceability across inheritance and repeated executions.

[Vampire]

• Provide unique IDs for all node info objects #1696 👈 (View in Graphite)
• master

This stack of pull requests is managed by Graphite. Learn more about stacking.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.17%. Comparing base (9c6342c) to head (e84a5c6).

Additional details and impacted files

@@            Coverage Diff            @@
##             master    #1696   +/-   ##
=========================================
  Coverage     82.16%   82.17%           
- Complexity     4822     4825    +3     
=========================================
  Files           472      472           
  Lines         15036    15043    +7     
  Branches       1905     1905           
=========================================
+ Hits          12355    12362    +7     
  Misses         1989     1989           
  Partials        692      692           

Files with missing lines	Coverage Δ
...va/org/spockframework/runtime/SpecInfoBuilder.java	99.27% <100.00%> (+0.01%)	⬆️
...rg/spockframework/runtime/model/IterationInfo.java	95.45% <100.00%> (+0.21%)	⬆️
...ava/org/spockframework/runtime/model/NodeInfo.java	100.00% <100.00%> (ø)

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[leonard84]

What is the use-case? And shouldn't we have something that is stable across invocations, otherwise it's use is severely limited.

[Vampire]

What is the use-case?

Well, everytime you need a unique ID during the test.
One use-case where I needed it is for example to put it as thread context map entry to log messages, so that you can identify log messages belonging to an iteration and test logging behavior in a multi-threaded SUT.

And shouldn't we have something that is stable across invocations, otherwise it's use is severely limited.

Well, the intended use by me was during one test run to relate things, without needing to build a unique ID yourself.
And for spec info, feature info and iteration info, it actually is stable across invocations, just not for "other" nodes.
Others being data provider info, field info, method info and parameter info.

Not sure whether we need stable unique ids for those. :-/

[Vampire]

Also, that they are stable across invocations for spec, feature, and iteration info is mainly a side-effect. The goal was to have something descriptive that can the also easily be readable for example in the log output where I intend to use it. :-D

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a63c91f1-c832-4fc3-b781-acb0c2b818c7

📥 Commits

Reviewing files that changed from the base of the PR and between 25912aa and e84a5c6.

📒 Files selected for processing (1)

• spock-core/src/main/java/org/spockframework/runtime/model/IterationInfo.java

🚧 Files skipped from review as they are similar to previous changes (1)

• spock-core/src/main/java/org/spockframework/runtime/model/IterationInfo.java

---

📝 Walkthrough

Walkthrough

Adds hierarchical unique identifiers across the node model: SpecInfo gets a spec-level ID (class name), FeatureInfo IDs are formed as specId.methodName, IterationInfo IDs as featureId[index], and NodeInfo gains a UUID-based uniqueId with accessors.

Changes

Cohort / File(s)	Summary
Core Model: NodeInfo spock-core/src/main/java/org/spockframework/runtime/model/NodeInfo.java	Added private uniqueId (UUID default), Javadoc, public String getUniqueId() and protected void setUniqueId(String); added java.util.UUID import.
Spec/Feature setters spock-core/src/main/java/org/spockframework/runtime/SpecInfo.java, spock-core/src/main/java/org/spockframework/runtime/model/FeatureInfo.java	Added void setUniqueId(String) methods to allow external assignment/propagation of unique IDs.
Builder initialization spock-core/src/main/java/org/spockframework/runtime/SpecInfoBuilder.java	Sets SpecInfo uniqueId (class name) during spec build and assigns each FeatureInfo a uniqueId formatted as specUniqueId.methodName when creating features.
Iteration IDs spock-core/src/main/java/org/spockframework/runtime/model/IterationInfo.java	Iteration constructor now sets uniqueId as String.format(Locale.ROOT, "%s[%d]", feature.getUniqueId(), iterationIndex) producing featureId[index].

Sequence Diagram(s)

sequenceDiagram
    participant Builder as SpecInfoBuilder
    participant Spec as SpecInfo
    participant Feature as FeatureInfo
    participant Iteration as IterationInfo

    Builder->>Spec: buildSpec(class)
    Spec->>Spec: setUniqueId(className)
    Builder->>Feature: createFeature(method)
    Feature->>Feature: setUniqueId(specUniqueId + "." + methodName)
    Feature->>Iteration: createIteration(index)
    Iteration->>Iteration: setUniqueId(featureUniqueId + "[" + index + "]")

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 I stitched IDs from class to test and thread,
A spec, some features, and indices in bed.
UUIDs hum, method names gleam,
Iterations hop into the scheme. ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: adding unique identifiers to NodeInfo and its subclasses (SpecInfo, FeatureInfo, IterationInfo).

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch unique-node-ids

📝 Coding Plan

• Generate coding plan for human review comments

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@spock-core/src/main/java/org/spockframework/runtime/model/IterationInfo.java`:
- Around line 33-35: The iteration ID is being built with locale-sensitive
String.format in the IterationInfo constructor
(setUniqueId(String.format("%s[%d]", ...))), which can produce locale-specific
digits; change this to a locale-independent construction—either use simple
String concatenation (feature.getUniqueId() + "[" + iterationIndex + "]") or
call String.format(Locale.ROOT, "%s[%d]", feature.getUniqueId(),
iterationIndex)—so setUniqueId always receives a stable, locale-neutral
identifier.

In `@spock-core/src/main/java/org/spockframework/runtime/model/NodeInfo.java`:
- Around line 38-52: The public mutator setUniqueId on NodeInfo exposes and
allows replacing the node's uniqueId (and setting null/duplicates); remove or
restrict that mutator and enforce one-time, non-null assignment instead: either
make setUniqueId non-public (protected or package-private) or eliminate it and
assign uniqueId during construction, and if you keep an assignment method ensure
it throws on null or if uniqueId is already set (idempotent write-protection).
Update references to getUniqueId, setUniqueId and the uniqueId field accordingly
so only internal/runtime code can set the ID and callers cannot overwrite it.
- Around line 23-31: NodeInfo currently eagerly allocates a UUID in the field
declaration and exposes a public setUniqueId, causing wasted allocations
(IterationInfo immediately replaces it) and allowing external mutation; change
uniqueId to not be eagerly generated (e.g. initialize to null and lazily create
on first getUniqueId OR require subclasses to supply a semantic id via
constructor), replace the public setUniqueId with a non-public API (make it
private/protected or remove it) and, if desired, implement a cheap per-execution
id generator (static AtomicLong counter) that subclasses or getUniqueId can use
to produce lightweight unique values; update NodeInfo.getUniqueId/setters and
constructors of SpecInfo, FeatureInfo, IterationInfo accordingly to ensure
immutability of the identifier.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d290a688-11d0-4184-abc2-413afcc836e3

📥 Commits

Reviewing files that changed from the base of the PR and between 9c6342c and 612a600.

📒 Files selected for processing (3)

• spock-core/src/main/java/org/spockframework/runtime/SpecInfoBuilder.java
• spock-core/src/main/java/org/spockframework/runtime/model/IterationInfo.java
• spock-core/src/main/java/org/spockframework/runtime/model/NodeInfo.java

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, locate and examine NodeInfo.java
find . -type f -name "NodeInfo.java" | head -20

Repository: spockframework/spock

Length of output: 137

---

🏁 Script executed:

# Check the full content of NodeInfo.java
if [ -f "spock-core/src/main/java/org/spockframework/runtime/model/NodeInfo.java" ]; then
  cat -n "spock-core/src/main/java/org/spockframework/runtime/model/NodeInfo.java"
else
  # Search for the file
  find . -name "NodeInfo.java" -type f
fi

Repository: spockframework/spock

Length of output: 3665

---

🏁 Script executed:

# Also check SpecInfoBuilder.java to see if it assigns semantic IDs
find . -type f -name "SpecInfoBuilder.java" | head -5

Repository: spockframework/spock

Length of output: 138

---

🏁 Script executed:

# Check IterationInfo.java
find . -type f -name "IterationInfo.java" | head -5

Repository: spockframework/spock

Length of output: 142

---

🏁 Script executed:

# Examine SpecInfoBuilder.java to find semantic ID assignments
cat -n "spock-core/src/main/java/org/spockframework/runtime/SpecInfoBuilder.java"

Repository: spockframework/spock

Length of output: 10958

---

🏁 Script executed:

# Examine IterationInfo.java 
cat -n "spock-core/src/main/java/org/spockframework/runtime/model/IterationInfo.java"

Repository: spockframework/spock

Length of output: 5336

---

🏁 Script executed:

# Search for setUniqueId calls to verify overwriting of uniqueId
rg "setUniqueId" -n --context 3

Repository: spockframework/spock

Length of output: 3267

---

Avoid eagerly generating a UUID in the base type.

SpecInfo, FeatureInfo, and IterationInfo each overwrite the default UUID with semantic IDs. On the per-iteration path, this causes unnecessary UUID allocation overhead—each IterationInfo constructor immediately discards the generated UUID. A per-execution counter would satisfy the documented uniqueness requirement without the allocation cost.

Additionally, the public setUniqueId() setter (lines 50–52) weakens the API contract by allowing arbitrary mutation of what should be an immutable identifier.

💡 One possible simplification

-import java.util.UUID;
+import java.util.concurrent.atomic.AtomicLong;
 
 public abstract class NodeInfo<P extends NodeInfo, R extends AnnotatedElement> {
-  private String uniqueId = UUID.randomUUID().toString();
+  private static final AtomicLong NEXT_ID = new AtomicLong();
+  private String uniqueId = "node-" + NEXT_ID.incrementAndGet();

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@spock-core/src/main/java/org/spockframework/runtime/model/NodeInfo.java`
around lines 23 - 31, NodeInfo currently eagerly allocates a UUID in the field
declaration and exposes a public setUniqueId, causing wasted allocations
(IterationInfo immediately replaces it) and allowing external mutation; change
uniqueId to not be eagerly generated (e.g. initialize to null and lazily create
on first getUniqueId OR require subclasses to supply a semantic id via
constructor), replace the public setUniqueId with a non-public API (make it
private/protected or remove it) and, if desired, implement a cheap per-execution
id generator (static AtomicLong counter) that subclasses or getUniqueId can use
to produce lightweight unique values; update NodeInfo.getUniqueId/setters and
constructors of SpecInfo, FeatureInfo, IterationInfo accordingly to ensure
immutability of the identifier.