Add duplicate subtask handling and tests
Change-Id: I08244cfb488ef576477ecd151a73ed4cb820eca4
diff --git a/src/main/java/com/googlesource/gerrit/plugins/task/TaskTree.java b/src/main/java/com/googlesource/gerrit/plugins/task/TaskTree.java
index 0b37f28..3dfda1f 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/task/TaskTree.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/task/TaskTree.java
@@ -30,10 +30,18 @@
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.ArrayList;
+import java.util.HashSet;
import java.util.LinkedList;
import java.util.List;
+import java.util.Set;
import org.eclipse.jgit.errors.ConfigInvalidException;
+/**
+ * Add structure to access the task definitions from the config as a tree.
+ *
+ * <p>This class is a "middle" representation of the task tree. The task config is represented as a
+ * lazily loaded tree, and much of the tree validity is enforced at this layer.
+ */
public class TaskTree {
protected static final String TASK_DIR = "task";
@@ -67,11 +75,20 @@
protected class NodeList {
protected LinkedList<Task> path = new LinkedList<>();
protected List<Node> nodes;
+ protected Set<String> names = new HashSet<>();
protected void addSubDefinitions(List<Task> tasks) {
for (Task task : tasks) {
- // path check detects looping definitions
- nodes.add(path.contains(task) ? null : new Node(task, path));
+ if (task != null && !path.contains(task) && names.add(task.name)) {
+ // path check above detects looping definitions
+ // names check above detects duplicate subtasks
+ try {
+ nodes.add(new Node(task, path));
+ continue;
+ } catch (Exception e) {
+ } // bad definition, handled below
+ }
+ nodes.add(null);
}
}
}
diff --git a/src/main/resources/Documentation/task_states.md b/src/main/resources/Documentation/task_states.md
index 01716cb..240d616 100644
--- a/src/main/resources/Documentation/task_states.md
+++ b/src/main/resources/Documentation/task_states.md
@@ -182,12 +182,17 @@
```
[task "No PASS criteria"]
applicable = is:open
+ fail-hint = Invalid without Pass criteria and without subtasks
[task "WAITING (subtask INVALID)"]
applicable = is:open
pass = is:open
subtask = Subtask INVALID
+[task "WAITING (subtask duplicate)"]
+ subtask = Subtask INVALID
+ subtask = Subtask INVALID
+
[task "WAITING (subtask missing)"]
applicable = is:open
pass = is:open
@@ -203,6 +208,7 @@
[task "Subtask INVALID"]
applicable = is:open
+ fail-hint = Use when an INVALID subtask is needed, not meant as a test case in itself
[task "NA Bad PASS query"]
applicable = -is:open
@@ -531,6 +537,22 @@
]
},
{
+ "hasPass" : false,
+ "name" : "WAITING (subtask duplicate)",
+ "status" : "WAITING",
+ "subTasks" : [
+ {
+ "hasPass" : false,
+ "name" : "Subtask INVALID",
+ "status" : "INVALID"
+ },
+ {
+ "name" : "UNKNOWN",
+ "status" : "INVALID"
+ }
+ ]
+ },
+ {
"hasPass" : true,
"name" : "WAITING (subtask missing)",
"status" : "WAITING",
diff --git a/test/all b/test/all
index 4e62515..840e11c 100644
--- a/test/all
+++ b/test/all
@@ -353,6 +353,24 @@
},
{
"applicable" : true,
+ "hasPass" : false,
+ "name" : "WAITING (subtask duplicate)",
+ "status" : "WAITING",
+ "subTasks" : [
+ {
+ "applicable" : true,
+ "hasPass" : false,
+ "name" : "Subtask INVALID",
+ "status" : "INVALID"
+ },
+ {
+ "name" : "UNKNOWN",
+ "status" : "INVALID"
+ }
+ ]
+ },
+ {
+ "applicable" : true,
"hasPass" : true,
"name" : "WAITING (subtask missing)",
"status" : "WAITING",
@@ -471,6 +489,24 @@
},
{
"applicable" : true,
+ "hasPass" : false,
+ "name" : "WAITING (subtask duplicate)",
+ "status" : "WAITING",
+ "subTasks" : [
+ {
+ "applicable" : true,
+ "hasPass" : false,
+ "name" : "Subtask INVALID",
+ "status" : "INVALID"
+ },
+ {
+ "name" : "UNKNOWN",
+ "status" : "INVALID"
+ }
+ ]
+ },
+ {
+ "applicable" : true,
"hasPass" : true,
"name" : "WAITING (subtask missing)",
"status" : "WAITING",
diff --git a/test/invalid b/test/invalid
index 528cad0..3347b60 100644
--- a/test/invalid
+++ b/test/invalid
@@ -74,6 +74,24 @@
},
{
"applicable" : true,
+ "hasPass" : false,
+ "name" : "WAITING (subtask duplicate)",
+ "status" : "WAITING",
+ "subTasks" : [
+ {
+ "applicable" : true,
+ "hasPass" : false,
+ "name" : "Subtask INVALID",
+ "status" : "INVALID"
+ },
+ {
+ "name" : "UNKNOWN",
+ "status" : "INVALID"
+ }
+ ]
+ },
+ {
+ "applicable" : true,
"hasPass" : true,
"name" : "WAITING (subtask missing)",
"status" : "WAITING",
@@ -180,6 +198,24 @@
},
{
"applicable" : true,
+ "hasPass" : false,
+ "name" : "WAITING (subtask duplicate)",
+ "status" : "WAITING",
+ "subTasks" : [
+ {
+ "applicable" : true,
+ "hasPass" : false,
+ "name" : "Subtask INVALID",
+ "status" : "INVALID"
+ },
+ {
+ "name" : "UNKNOWN",
+ "status" : "INVALID"
+ }
+ ]
+ },
+ {
+ "applicable" : true,
"hasPass" : true,
"name" : "WAITING (subtask missing)",
"status" : "WAITING",
diff --git a/test/invalid-applicable b/test/invalid-applicable
index c078c92..b1f9f0b 100644
--- a/test/invalid-applicable
+++ b/test/invalid-applicable
@@ -47,6 +47,22 @@
]
},
{
+ "hasPass" : false,
+ "name" : "WAITING (subtask duplicate)",
+ "status" : "WAITING",
+ "subTasks" : [
+ {
+ "hasPass" : false,
+ "name" : "Subtask INVALID",
+ "status" : "INVALID"
+ },
+ {
+ "name" : "UNKNOWN",
+ "status" : "INVALID"
+ }
+ ]
+ },
+ {
"hasPass" : true,
"name" : "WAITING (subtask missing)",
"status" : "WAITING",
diff --git a/test/preview b/test/preview
index 9394637..2989f99 100644
--- a/test/preview
+++ b/test/preview
@@ -30,6 +30,24 @@
},
{
"applicable" : true,
+ "hasPass" : false,
+ "name" : "WAITING (subtask duplicate)",
+ "status" : "WAITING",
+ "subTasks" : [
+ {
+ "applicable" : true,
+ "hasPass" : false,
+ "name" : "Subtask INVALID",
+ "status" : "INVALID"
+ },
+ {
+ "name" : "UNKNOWN",
+ "status" : "INVALID"
+ }
+ ]
+ },
+ {
+ "applicable" : true,
"hasPass" : true,
"name" : "WAITING (subtask missing)",
"status" : "WAITING",
@@ -183,6 +201,24 @@
},
{
"applicable" : true,
+ "hasPass" : false,
+ "name" : "WAITING (subtask duplicate)",
+ "status" : "WAITING",
+ "subTasks" : [
+ {
+ "applicable" : true,
+ "hasPass" : false,
+ "name" : "Subtask INVALID",
+ "status" : "INVALID"
+ },
+ {
+ "name" : "UNKNOWN",
+ "status" : "INVALID"
+ }
+ ]
+ },
+ {
+ "applicable" : true,
"hasPass" : true,
"name" : "WAITING (subtask missing)",
"status" : "WAITING",
diff --git a/test/preview.invalid b/test/preview.invalid
index 16e0426..9521061 100644
--- a/test/preview.invalid
+++ b/test/preview.invalid
@@ -30,6 +30,24 @@
},
{
"applicable" : true,
+ "hasPass" : false,
+ "name" : "WAITING (subtask duplicate)",
+ "status" : "WAITING",
+ "subTasks" : [
+ {
+ "applicable" : true,
+ "hasPass" : false,
+ "name" : "Subtask INVALID",
+ "status" : "INVALID"
+ },
+ {
+ "name" : "UNKNOWN",
+ "status" : "INVALID"
+ }
+ ]
+ },
+ {
+ "applicable" : true,
"hasPass" : true,
"name" : "WAITING (subtask missing)",
"status" : "WAITING",
@@ -136,6 +154,24 @@
},
{
"applicable" : true,
+ "hasPass" : false,
+ "name" : "WAITING (subtask duplicate)",
+ "status" : "WAITING",
+ "subTasks" : [
+ {
+ "applicable" : true,
+ "hasPass" : false,
+ "name" : "Subtask INVALID",
+ "status" : "INVALID"
+ },
+ {
+ "name" : "UNKNOWN",
+ "status" : "INVALID"
+ }
+ ]
+ },
+ {
+ "applicable" : true,
"hasPass" : true,
"name" : "WAITING (subtask missing)",
"status" : "WAITING",
