Add presubmit linter hook for failures.txt expectation files
This change is primarily targeted to reduce the number of failures that
can happen when failure expectation files for art buildbot are
modified, by adding a linter hook to check files ending with
'_failures.txt'. The example of the failure can be found here:
https://ci.chromium.org/ui/p/art/builders/ci/fugu-ndebug/2433/overview
Linter is written in java because vogar uses gson library with lenient
mode enabled to parse JSON expectations. This mode allows C-style
comments, unquoted keys and values, top level values of any type and
some other differences which make use of default jsonlint hook or a
simpler python script impossible.
This hook is filtering all changed files ending with '_failures.txt',
and passes them to a linter that parses those files with lenient mode;
and fails if JSON is malformed. Linter is compiled from source in a
temporary directory which is removed after checks.
Bug: 235103837
Test: art/tools/check_presubmit_json_expectations.sh \
$ANDROID_BUILD_TOP art/tools/*_failures.txt
Test: $ANDROID_BUILD_TOP/tools/repohooks/pre-upload.py
Change-Id: I8fc10bf39d1b8ced0ae88152a8e3a0dbf8e44035
diff --git a/PREUPLOAD.cfg b/PREUPLOAD.cfg
index b8ee3c5..2182991 100644
--- a/PREUPLOAD.cfg
+++ b/PREUPLOAD.cfg
@@ -7,6 +7,8 @@
# so we don't need the custom runtests script and this check.
check_libnativebridge_test_field = libnativebridge/tests/preupload_check_test_tag.sh ${PREUPLOAD_COMMIT_MESSAGE} ${PREUPLOAD_FILES}
+check_expectation_jsons = tools/check_presubmit_json_expectations.sh ${REPO_ROOT} ${PREUPLOAD_FILES}
+
[Builtin Hooks]
cpplint = true
bpfmt = true
diff --git a/tools/PresubmitJsonLinter.java b/tools/PresubmitJsonLinter.java
new file mode 100644
index 0000000..334d200
--- /dev/null
+++ b/tools/PresubmitJsonLinter.java
@@ -0,0 +1,197 @@
+/*
+ * Copyright (C) 2022 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import com.google.gson.stream.JsonReader;
+import java.io.FileNotFoundException;
+import java.io.FileReader;
+import java.io.IOException;
+import java.util.HashSet;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Set;
+import java.util.regex.Pattern;
+import java.util.regex.PatternSyntaxException;
+
+/**
+ * Pre upload hook that ensures art-buildbot expectation files (files under //art/tools ending with
+ * "_failures.txt", e.g. //art/tools/libcore_failures.txt) are well-formed json files.
+ *
+ * It makes basic validation of the keys but does not cover all the cases. Parser structure is
+ * based on external/vogar/src/vogar/ExpectationStore.java.
+ *
+ * Hook is set up in //art/PREUPLOAD.cfg See also //tools/repohooks/README.md
+ */
+class PresubmitJsonLinter {
+
+ private static final int FLAGS = Pattern.MULTILINE | Pattern.DOTALL;
+ private static final Set<String> RESULTS = new HashSet<>();
+
+ static {
+ RESULTS.addAll(List.of(
+ "UNSUPPORTED",
+ "COMPILE_FAILED",
+ "EXEC_FAILED",
+ "EXEC_TIMEOUT",
+ "ERROR",
+ "SUCCESS"
+ ));
+ }
+
+ public static void main(String[] args) {
+ for (String arg : args) {
+ info("Checking " + arg);
+ checkExpectationFile(arg);
+ }
+ }
+
+ private static void info(String message) {
+ System.err.println(message);
+ }
+
+ private static void error(String message) {
+ System.err.println(message);
+ System.exit(1);
+ }
+
+ private static void checkExpectationFile(String arg) {
+ JsonReader reader;
+ try {
+ reader = new JsonReader(new FileReader(arg));
+ } catch (FileNotFoundException e) {
+ error("File '" + arg + "' is not found");
+ return;
+ }
+ reader.setLenient(true);
+ try {
+ reader.beginArray();
+ while (reader.hasNext()) {
+ readExpectation(reader);
+ }
+ reader.endArray();
+ } catch (IOException e) {
+ error("Malformed json: " + reader);
+ }
+ }
+
+ private static void readExpectation(JsonReader reader) throws IOException {
+ Set<String> names = new LinkedHashSet<String>();
+ Set<String> tags = new LinkedHashSet<String>();
+ boolean readResult = false;
+ boolean readDescription = false;
+
+ reader.beginObject();
+ while (reader.hasNext()) {
+ String name = reader.nextName();
+ switch (name) {
+ case "result":
+ String result = reader.nextString();
+ if (!RESULTS.contains(result)) {
+ error("Invalid 'result' value: '" + result +
+ "'. Expected one of " + String.join(", ", RESULTS) +
+ ". " + reader);
+ }
+ readResult = true;
+ break;
+ case "substring": {
+ try {
+ Pattern.compile(
+ ".*" + Pattern.quote(reader.nextString()) + ".*", FLAGS);
+ } catch (PatternSyntaxException e) {
+ error("Malformed 'substring' value: " + reader);
+ }
+ }
+ case "pattern": {
+ try {
+ Pattern.compile(reader.nextString(), FLAGS);
+ } catch (PatternSyntaxException e) {
+ error("Malformed 'pattern' value: " + reader);
+ }
+ break;
+ }
+ case "failure":
+ names.add(reader.nextString());
+ break;
+ case "description":
+ reader.nextString();
+ readDescription = true;
+ break;
+ case "name":
+ names.add(reader.nextString());
+ break;
+ case "names":
+ readStrings(reader, names);
+ break;
+ case "tags":
+ readStrings(reader, tags);
+ break;
+ case "bug":
+ reader.nextLong();
+ break;
+ case "modes":
+ readModes(reader);
+ break;
+ case "modes_variants":
+ readModesAndVariants(reader);
+ break;
+ default:
+ error("Unknown key '" + name + "' in expectations file");
+ reader.skipValue();
+ break;
+ }
+ }
+ reader.endObject();
+
+ if (names.isEmpty()) {
+ error("Missing 'name' or 'failure' key in " + reader);
+ }
+ if (!readResult) {
+ error("Missing 'result' key in " + reader);
+ }
+ if (!readDescription) {
+ error("Missing 'description' key in " + reader);
+ }
+ }
+
+ private static void readStrings(JsonReader reader, Set<String> output) throws IOException {
+ reader.beginArray();
+ while (reader.hasNext()) {
+ output.add(reader.nextString());
+ }
+ reader.endArray();
+ }
+
+ private static void readModes(JsonReader reader) throws IOException {
+ reader.beginArray();
+ while (reader.hasNext()) {
+ reader.nextString();
+ }
+ reader.endArray();
+ }
+
+ /**
+ * Expected format: mode_variants: [["host", "X32"], ["host", "X64"]]
+ */
+ private static void readModesAndVariants(JsonReader reader) throws IOException {
+ reader.beginArray();
+ while (reader.hasNext()) {
+ reader.beginArray();
+ reader.nextString();
+ reader.nextString();
+ reader.endArray();
+ }
+ reader.endArray();
+ }
+}
\ No newline at end of file
diff --git a/tools/check_presubmit_json_expectations.sh b/tools/check_presubmit_json_expectations.sh
new file mode 100755
index 0000000..ecb1e3e
--- /dev/null
+++ b/tools/check_presubmit_json_expectations.sh
@@ -0,0 +1,50 @@
+#!/bin/bash
+#
+# Copyright (C) 2022 The Android Open Source Project
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+set -e
+
+REPO_ROOT="$1"
+
+FILES_TO_CHECK=()
+for i in "${@:2}"; do
+ if [[ $i == *_failures.txt ]]; then
+ FILES_TO_CHECK+=($i)
+ fi
+done
+
+# if no libcore_*_failures.txt files were changed
+if [ ${#FILES_TO_CHECK[@]} -eq 0 ]; then
+ exit 0
+fi
+
+TMP_DIR=`mktemp -d`
+# check if tmp dir was created
+if [[ ! "$TMP_DIR" || ! -d "$TMP_DIR" ]]; then
+ echo "Could not create temp dir"
+ exit 1
+fi
+
+function cleanup {
+ rm -rf "$TMP_DIR"
+}
+
+# register the cleanup function to be called on the EXIT signal
+trap cleanup EXIT
+
+GSON_JAR="${REPO_ROOT}/external/caliper/lib/gson-2.2.2.jar"
+
+javac --class-path "$GSON_JAR" "${REPO_ROOT}/art/tools/PresubmitJsonLinter.java" -d "$TMP_DIR"
+java --class-path "$TMP_DIR:$GSON_JAR" PresubmitJsonLinter "${FILES_TO_CHECK[@]}"