Skip to content

Provide unique IDs for all node info objects #1696

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

Open
Vampire wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Provide unique IDs for all node info objects #1696

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
53bcc13
Provide unique IDs for all node info objects
Vampire Jun 10, 2023
0f36696
Review Feedback Vol. 1
Vampire Mar 13, 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
2 changes: 2 additions & 0 deletions spock-core/src/main/java/org/spockframework/runtime/SpecInfoBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ private void buildSpec() {
SpecUtil.checkIsSpec(clazz);

SpecMetadata metadata = clazz.getAnnotation(SpecMetadata.class);
spec.setUniqueId(clazz.getName());
spec.setParent(null);
spec.setPackage(ReflectionUtil.getPackageName(clazz));
spec.setName(clazz.getSimpleName());
Expand Down Expand Up @@ -115,6 +116,7 @@ private void buildFeatures() {

private FeatureInfo createFeature(Method method, FeatureMetadata featureMetadata) {
FeatureInfo feature = new FeatureInfo();
feature.setUniqueId(String.format("%s.%s", spec.getUniqueId(), method.getName()));
feature.setParent(spec);
feature.setName(featureMetadata.name());
feature.setLine(featureMetadata.line());
Expand Down
1 change: 1 addition & 0 deletions spock-core/src/main/java/org/spockframework/runtime/model/IterationInfo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ public class IterationInfo extends NodeInfo<FeatureInfo, AnnotatedElement> imple
private String displayName;

public IterationInfo(FeatureInfo feature, int iterationIndex, Object[] dataValues, int estimatedNumIterations) {
setUniqueId(String.format(Locale.ROOT, "%s[%d]", feature.getUniqueId(), iterationIndex));
setParent(feature);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
this.iterationIndex = iterationIndex;
this.dataValues = dataValues;
Expand Down
18 changes: 18 additions & 0 deletions spock-core/src/main/java/org/spockframework/runtime/model/NodeInfo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,19 +20,37 @@

import java.lang.annotation.Annotation;
import java.lang.reflect.AnnotatedElement;
import java.util.UUID;

/**
* Base class for runtime information about an element in a Spock specification.
*
* @author Peter Niederwieser
*/
public abstract class NodeInfo<P extends NodeInfo, R extends AnnotatedElement> {
private String uniqueId = UUID.randomUUID().toString();
Comment on lines +23 to +31
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Mar 13, 2026

Choose a reason for hiding this comment

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

⚠️ 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.

private String name;
private int line = -1;
private P parent;
private R reflection;
private Object metadata;

/**
* A unique ID for this node during the current execution. Although some subclasses might
* provide semantic IDs, and the default implementation uses UUIDs, no semantic or format
* or maximum length is guaranteed. The only thing that should be assumed is, that the ID
* is unique across all nodes during the same invocation.
*
* @return the unique ID for this node
*/
public String getUniqueId() {
return uniqueId;
}

public void setUniqueId(String uniqueId) {
this.uniqueId = uniqueId;
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

public String getName() {
return name;
}
Expand Down
Loading