[appengine] Fix a bug in computing the base name
The code could not handle names that do not match co19 or standard SDK
test names. This fix changes it to return a failure (and better error
message) instead of crashing.
Change-Id: Id8045843b60b3a6bd13b74d812efe84ece2b86bc
Reviewed-on: https://dart-review.googlesource.com/c/dart_ci/+/202020
Auto-Submit: Karl Klose <karlklose@google.com>
Reviewed-by: William Hesse <whesse@google.com>
Commit-Queue: William Hesse <whesse@google.com>
diff --git a/appengine/bin/server.dart b/appengine/bin/server.dart
index 315f1e3..3074b63 100644
--- a/appengine/bin/server.dart
+++ b/appengine/bin/server.dart
@@ -125,7 +125,10 @@
if (source != null) {
return redirectTemporary(request, source.toString());
} else {
- return notFound(request);
+ return notFound(request,
+ message: "No rules found that match test name '$testName'."
+ " If you think that this test name should work, please send a"
+ " message to dart-engprod@.");
}
} catch (e) {
throw UserVisibleFailure(e.toString());
@@ -138,8 +141,11 @@
return request.response.close();
}
-Future<void> notFound(HttpRequest request) {
+Future<void> notFound(HttpRequest request, {String message}) {
request.response.statusCode = HttpStatus.notFound;
+ if (message != null) {
+ request.response.write(message);
+ }
return request.response.close();
}
diff --git a/appengine/lib/src/test_source.dart b/appengine/lib/src/test_source.dart
index 0e1d90a..54d6296 100644
--- a/appengine/lib/src/test_source.dart
+++ b/appengine/lib/src/test_source.dart
@@ -50,7 +50,7 @@
final regExp = (suite == 'co19' || suite == 'co19_2')
? RegExp(r"t[0-9]{2,3}$")
: RegExp(r"_test$");
- for (var i = 0; i <= 2; i++) {
+ for (var i = 0; i <= 2 && parts.isNotEmpty; i++) {
if (regExp.hasMatch(parts.last)) {
return parts.join('/');
} else {
diff --git a/appengine/test/create_source_tests.dart b/appengine/test/create_source_tests.dart
index 037b2a5..de27723 100644
--- a/appengine/test/create_source_tests.dart
+++ b/appengine/test/create_source_tests.dart
@@ -44,7 +44,9 @@
testNames.putIfAbsent(key, () => testName);
}
- var results = {};
+ Map<String, Map<String, String>> results = {
+ 'suite/not_a_basename': {"true": null, "false": null},
+ };
for (var name in testNames.values) {
results[name] = {};
for (var gob in [true, false]) {
diff --git a/appengine/test/test_source_test.dart b/appengine/test/test_source_test.dart
index 8de03e0..baa7bcc 100644
--- a/appengine/test/test_source_test.dart
+++ b/appengine/test/test_source_test.dart
@@ -22,21 +22,21 @@
for (final name in testData.keys) {
for (final useGob in [true, false]) {
final expected = testData[name][useGob.toString()];
- if (expected == null) {
+ if (!testData[name].keys.toSet().containsAll(["true", "false"])) {
throw 'Invalid test data for $name/$useGob';
}
var actual;
var url;
try {
- url = (await computeTestSource(revision, name, useGob)).toString();
- actual = url.toString();
+ url = await computeTestSource(revision, name, useGob);
+ actual = url?.toString();
} catch (e) {
- actual = e.toString();
+ actual = "error: $e";
}
if (expected != actual) {
throw Exception("Expected \n'$expected', found\n'$actual'\n");
}
- if (verifyTargetExists) {
+ if (verifyTargetExists && url != null) {
final response = await http.head(url);
if (response.statusCode != HttpStatus.ok) {
throw Exception("Can't find target: $url");
@@ -49,6 +49,7 @@
// This data can be generated by running
// 'dart run create_source_tests.dart <file with test names>'
const testData = {
+ "suite/not_a_basename": {"true": null, "false": null},
"co19/Language/Classes/Abstract_Instance_Members/inherited_t01": {
"true":
"https://github.com/dart-lang/co19/blob/055b5c984613ec1b8ef76516db3ea99fee63acb9/Language/Classes/Abstract_Instance_Members/inherited_t01.dart",
