Query pending checks: Make is:<STATE> operators consistent
Some operators require an underscore in the state name (e.g.
'NOT_STARTED', 'NOT_RELEVANT'), while others don't accept it
('INPROGRESS'). This is because the former are mapped to the CheckState
enum, and the latter isn't. From a users perspective this difference is
not obvious. This is why we now support both forms, with and without
underscore, in all cases. This means 'NOTSTARTED', 'NOTRELEVANT' and
'IN_PROGRESS' are working now too.
Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: Ic6352306cd196751af9d6a2728433d312eb5f361
diff --git a/java/com/google/gerrit/plugins/checks/index/CheckQueryBuilder.java b/java/com/google/gerrit/plugins/checks/index/CheckQueryBuilder.java
index 9cdcb6d..ff6d1f5 100644
--- a/java/com/google/gerrit/plugins/checks/index/CheckQueryBuilder.java
+++ b/java/com/google/gerrit/plugins/checks/index/CheckQueryBuilder.java
@@ -43,7 +43,7 @@
@Operator
public Predicate<Check> is(String value) throws QueryParseException {
- if ("inprogress".equalsIgnoreCase(value)) {
+ if ("inprogress".equalsIgnoreCase(value) || "in_progress".equalsIgnoreCase(value)) {
return Predicate.or(
Arrays.stream(CheckState.values())
.filter(CheckState::isInProgress)
diff --git a/java/com/google/gerrit/plugins/checks/index/CheckStatePredicate.java b/java/com/google/gerrit/plugins/checks/index/CheckStatePredicate.java
index 326c02a..74cca68 100644
--- a/java/com/google/gerrit/plugins/checks/index/CheckStatePredicate.java
+++ b/java/com/google/gerrit/plugins/checks/index/CheckStatePredicate.java
@@ -31,7 +31,18 @@
}
public static Optional<CheckStatePredicate> tryParse(String value) {
- return Enums.getIfPresent(CheckState.class, value).toJavaUtil().map(CheckStatePredicate::new);
+ Optional<CheckStatePredicate> p =
+ Enums.getIfPresent(CheckState.class, value).toJavaUtil().map(CheckStatePredicate::new);
+ if (p.isPresent()) {
+ return p;
+ }
+
+ for (CheckState checkState : CheckState.values()) {
+ if (checkState.name().replace("_", "").equalsIgnoreCase(value)) {
+ return Optional.of(new CheckStatePredicate(checkState));
+ }
+ }
+ return Optional.empty();
}
private final CheckState checkState;
diff --git a/javatests/com/google/gerrit/plugins/checks/acceptance/api/QueryPendingChecksIT.java b/javatests/com/google/gerrit/plugins/checks/acceptance/api/QueryPendingChecksIT.java
index 32d41ee..0a58a79 100644
--- a/javatests/com/google/gerrit/plugins/checks/acceptance/api/QueryPendingChecksIT.java
+++ b/javatests/com/google/gerrit/plugins/checks/acceptance/api/QueryPendingChecksIT.java
@@ -303,6 +303,14 @@
} else {
assertThat(pendingChecks).isEmpty();
}
+
+ pendingChecks =
+ queryPendingChecks(String.format("checker:\"%s\" is:in_progress", checkerUuid));
+ if (checkState.isInProgress()) {
+ assertThat(pendingChecks).hasSize(1);
+ } else {
+ assertThat(pendingChecks).isEmpty();
+ }
}
}
@@ -367,12 +375,42 @@
}
@Test
+ public void queryPendingChecksForSpecifiedStateUnderscoreCanBeOmitted() throws Exception {
+ CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
+
+ // Create the check once so that in the for-loop we can always update an existing check, rather
+ // than needing to check if the check already exists and then depending on this either create or
+ // update it.
+ checkOperations
+ .newCheck(CheckKey.create(project, patchSetId, checkerUuid))
+ .setState(CheckState.NOT_STARTED)
+ .upsert();
+
+ for (CheckState checkState : CheckState.values()) {
+ checkOperations
+ .check(CheckKey.create(project, patchSetId, checkerUuid))
+ .forUpdate()
+ .setState(checkState)
+ .upsert();
+
+ List<PendingChecksInfo> pendingChecks =
+ queryPendingChecks(
+ String.format(
+ "checker:\"%s\" is:%s", checkerUuid, checkState.name().replace("_", "")));
+ assertThat(pendingChecks).hasSize(1);
+ }
+ }
+
+ @Test
public void queryPendingChecksForSpecifiedStateDifferentCases() throws Exception {
CheckerUuid checkerUuid = checkerOperations.newChecker().repository(project).create();
assertThat(queryPendingChecks(buildQueryString(checkerUuid) + " state:NOT_STARTED")).hasSize(1);
assertThat(queryPendingChecks(buildQueryString(checkerUuid) + " state:not_started")).hasSize(1);
assertThat(queryPendingChecks(buildQueryString(checkerUuid) + " state:NoT_StArTeD")).hasSize(1);
+ assertThat(queryPendingChecks(buildQueryString(checkerUuid) + " state:NOTSTARTED")).hasSize(1);
+ assertThat(queryPendingChecks(buildQueryString(checkerUuid) + " state:notstarted")).hasSize(1);
+ assertThat(queryPendingChecks(buildQueryString(checkerUuid) + " state:NoTStArTeD")).hasSize(1);
}
@Test
diff --git a/src/main/resources/Documentation/rest-api-pending-checks.md b/src/main/resources/Documentation/rest-api-pending-checks.md
index 8924c95..c28c0a1 100644
--- a/src/main/resources/Documentation/rest-api-pending-checks.md
+++ b/src/main/resources/Documentation/rest-api-pending-checks.md
@@ -121,8 +121,9 @@
* <a id="checker-operator"></a> `checker:'CHECKER_UUID'`:
Matches checks of the checker with the UUID 'CHECKER_UUID'.
* <a id="is-operator"></a> `is:'STATE'`:
- Matches checks with the state 'STATE'.
-* <a id="is-inprogress-operator"></a> `is:inprogress`:
+ Matches checks with the state 'STATE'. Underscores in the state name
+ can be omitted.
+* <a id="is-inprogress-operator"></a> `is:inprogress`/`is:in_progress`:
Matches checks with non-final states (`NOT_STARTED`, `SCHEDULED` and
`RUNNING`).
* <a id="state-operator"></a> `state:'STATE'`:
