Support Pass/Fail tasks
Pass/Fail tasks are defined by only defining a fail criteria. These
tasks are never ready, if they would otherwise be ready, they will PASS
instead. This new feature will allow a single pass/fail criteria to be
defined in lieu of defining both a pass and a complemented fail
criteria. This single fail query definition feature makes defining such
tasks less error-prone all while making their evaluation by the plugin
non-racy.
Change-Id: If14fe2df89d396a4bd057c10591b3ccc392398df
diff --git a/src/main/java/com/googlesource/gerrit/plugins/task/TaskAttributeFactory.java b/src/main/java/com/googlesource/gerrit/plugins/task/TaskAttributeFactory.java
index 6857203..21a5baa 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/task/TaskAttributeFactory.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/task/TaskAttributeFactory.java
@@ -249,7 +249,7 @@
protected Status getStatusWithExceptions(ChangeData c, Task task, TaskAttribute a)
throws OrmException, QueryParseException {
- if (task.pass == null && a.subTasks == null) {
+ if (isAllNull(task.pass, task.fail, a.subTasks)) {
// A leaf task has no defined subtasks.
boolean hasDefinedSubtasks =
!(task.subTasks.isEmpty()
@@ -257,36 +257,43 @@
&& task.subTasksExternals.isEmpty());
if (hasDefinedSubtasks) {
// Remove 'Grouping" tasks (tasks with subtasks but no PASS
- // criteria) from the output if none of their subtasks are
- // applicable. i.e. grouping tasks only really apply if at
+ // or FAIL criteria) from the output if none of their subtasks
+ // are applicable. i.e. grouping tasks only really apply if at
// least one of their subtasks apply.
return null;
}
- // A leaf configuration without a PASS criteria is a missconfiguration.
- // Either someone forgot to add subtasks, or they forgot to add
- // the PASS criteria.
+ // A leaf configuration without a PASS or FAIL criteria is a
+ // missconfiguration. Either someone forgot to add subtasks, or
+ // they forgot to add a PASS or FAIL criteria.
return Status.INVALID;
}
- if (task.fail != null && match(c, task.fail)) {
- // A FAIL definition is meant to be a hard blocking criteria
- // (like a CodeReview -2). Thus, if hard blocked, it is
- // irrelevant what the subtask states, or the pass criteria are.
- //
- // It is also important that FAIL be useable to indicate that
- // the task has actually executed. Thus subtask status,
- // including a subtask FAIL should not appear as a FAIL on the
- // parent task. This means that this is should be the only path
- // to make a task have a FAIL status.
- return Status.FAIL;
+ if (task.fail != null) {
+ if (match(c, task.fail)) {
+ // A FAIL definition is meant to be a hard blocking criteria
+ // (like a CodeReview -2). Thus, if hard blocked, it is
+ // irrelevant what the subtask states, or the PASS criteria are.
+ //
+ // It is also important that FAIL be useable to indicate that
+ // the task has actually executed. Thus subtask status,
+ // including a subtask FAIL should not appear as a FAIL on the
+ // parent task. This means that this is should be the only path
+ // to make a task have a FAIL status.
+ return Status.FAIL;
+ }
+ if (task.pass == null) {
+ // A task with a FAIL but no PASS criteria is a PASS-FAIL task
+ // (they are never "READY"). It didn't fail, so pass.
+ return Status.PASS;
+ }
}
if (a.subTasks != null && !isAll(a.subTasks, Status.PASS)) {
// It is possible for a subtask's PASS criteria to change while
// a parent task is executing, or even after the parent task
// completes. This can result in the parent PASS criteria being
- // met while one or more of its subtasks no longer meets its pass
- // criteria (the subtask may now even meet a fail criteria). We
+ // met while one or more of its subtasks no longer meets its PASS
+ // criteria (the subtask may now even meet a FAIL criteria). We
// never want the parent task to reflect a PASS criteria in these
// cases, thus we can safely return here without ever evaluating
// the task's PASS criteria.
@@ -321,4 +328,13 @@
}
return ((Matchable) cqb.parse(query)).match(c);
}
+
+ protected static boolean isAllNull(Object... vals) {
+ for (Object val : vals) {
+ if (val != null) {
+ return false;
+ }
+ }
+ return true;
+ }
}
diff --git a/src/main/resources/Documentation/task.md b/src/main/resources/Documentation/task.md
index 3f5c606..46ada9b 100644
--- a/src/main/resources/Documentation/task.md
+++ b/src/main/resources/Documentation/task.md
@@ -97,7 +97,9 @@
: This key defines a query that is used to determine whether a task is
currently in-progress or not. A CI system may use this to ensure that it
-only runs one verification instance for a specific change.
+only runs one verification instance for a specific change. Either a pass
+or fail key is mandatory for leaf tasks. A task with a fail criteria,
+but no pass criteria, will pass if it otherwise would be ready.
Example:
```
@@ -107,10 +109,10 @@
`pass`
: This key defines a query that is used to determine whether a task has
-already executed and passed for each change. This key is mandatory for
-leaf tasks. Tasks with no defined pass criteria and with defined subtasks
-are valid, but they are only applicable when at least one subtask is
-applicable.
+already executed and passed for each change. Either a pass or fail
+key is mandatory for leaf tasks. Tasks with no defined pass criteria and
+with defined subtasks are valid, but they are only applicable when at least
+one subtask is applicable.
Example:
```
diff --git a/src/main/resources/Documentation/task_states.md b/src/main/resources/Documentation/task_states.md
index b405c19..4563390 100644
--- a/src/main/resources/Documentation/task_states.md
+++ b/src/main/resources/Documentation/task_states.md
@@ -19,6 +19,14 @@
fail = is:open
pass = is:open
+[root "Root PASS-fail"]
+ applicable = is:open
+ fail = NOT is:open
+
+[root "Root pass-FAIL"]
+ applicable = is:open
+ fail = is:open
+
[root "Root grouping PASS (subtask PASS)"]
applicable = is:open
subtask = Subtask PASS
@@ -201,6 +209,14 @@
"status" : "FAIL"
},
{
+ "name" : "Root PASS-fail",
+ "status" : "PASS"
+ },
+ {
+ "name" : "Root pass-FAIL",
+ "status" : "FAIL"
+ },
+ {
"name" : "Root grouping PASS (subtask PASS)",
"status" : "PASS",
"subTasks" : [
