[infra] Add a script to narrow blamelists in the result feed

This script takes a list of test results and identifies blamelists
in the result feed data that include the commit of the test results
and tries to narrow the blamelist, if possible.

This CL also adds a small library to use the firestore REST API,
which contains mostly the functionality used in the script, but
should be easy enough to extend for other scripts.

Change-Id: If3c8272438e2a9bbf24891d9f5b62c342ea77cc6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/153966
Commit-Queue: Karl Klose <karlklose@google.com>
Reviewed-by: William Hesse <whesse@google.com>

pkg/test_runner/lib/bot_results.dart

@@ -14,6 +14,52 @@ import 'package:pool/pool.dart';
 /// The path to the gsutil script.
 String gsutilPy;
 
+// TODO(karlklose):  Update this class with all fields that
+// are used in pkg/test_runner and the tools/bots scripts and include
+// validation (in particular for fields from extend_results.dart, that are
+// optional but expected in some contexts and should always be all or nothing).
+class Result {
+  final String configuration;
+  final String expectation;
+  final bool matches;
+  final String name;
+  final String outcome;
+  final bool changed;
+  final String commitHash;
+  // TODO(karlklose): this field is unnecessary with extended results and
+  // should be removed.
+  final bool flaked;
+  final bool isFlaky;
+  final String previousOutcome;
+
+  Result(
+      this.configuration,
+      this.name,
+      this.outcome,
+      this.expectation,
+      this.matches,
+      this.changed,
+      this.commitHash,
+      this.isFlaky,
+      this.previousOutcome,
+      [this.flaked = false]);
+
+  Result.fromMap(Map<String, dynamic> map, [Map<String, dynamic> flakinessData])
+      : configuration = map["configuration"] as String,
+        name = map["name"] as String,
+        outcome = map["result"] as String,
+        expectation = map["expected"] as String,
+        matches = map["matches"] as bool,
+        changed = map["changed"] as bool,
+        commitHash = map["commit_hash"] as String,
+        isFlaky = map["flaky"] as bool,
+        previousOutcome = map["previous_result"] as String,
+        flaked = flakinessData != null &&
+            (flakinessData["outcomes"] as List).contains(map["result"]);
+
+  String get key => "$configuration:$name";
+}
+
 /// Cloud storage location containing results.
 const testResultsStoragePath = "gs://dart-test-results/builders";
 

tools/bots/compare_results.dart

@@ -13,29 +13,6 @@ import 'dart:io';
 import 'package:args/args.dart';
 import 'package:test_runner/bot_results.dart';
 
-class Result {
-  final String configuration;
-  final String name;
-  final String outcome;
-  final String expectation;
-  final bool matches;
-  final bool flaked;
-
-  Result(this.configuration, this.name, this.outcome, this.expectation,
-      this.matches, this.flaked);
-
-  Result.fromMap(Map<String, dynamic> map, Map<String, dynamic> flakinessData)
-      : configuration = map["configuration"],
-        name = map["name"],
-        outcome = map["result"],
-        expectation = map["expected"],
-        matches = map["matches"],
-        flaked = flakinessData != null &&
-            flakinessData["outcomes"].contains(map["result"]);
-
-  String get key => "$configuration:$name";
-}
-
 class Event {
   final Result before;
   final Result after;

tools/bots/lib/src/firestore.dart

@@ -0,0 +1,217 @@
+// Copyright (c) 2020, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'dart:convert' show jsonDecode, jsonEncode;
+import 'dart:io' show File, HttpStatus;
+
+import 'package:http/http.dart' as http;
+
+Future<String> readGcloudAuthToken(String path) async {
+  String token = await File(path).readAsString();
+  return token.split("\n").first;
+}
+
+/// Helper class to access the Firestore REST API.
+///
+/// This class is not a complete implementation of the Firestore REST protocol
+/// and is only meant to support the operations required by scripts in
+/// tools/bots.
+class FirestoreDatabase {
+  final http.Client _client = http.Client();
+  final String _authToken;
+  final String _project;
+
+  /// The current transaction ID in base64 (or `null`)
+  String _currentTransaction;
+
+  /// Returns the current transaction escaped to be useable as part of a URI.
+  String get _escapedCurrentTransaction {
+    return Uri.encodeFull(_currentTransaction)
+        // The Firestore API does not accept '+' in URIs
+        .replaceAll("+", "%2B");
+  }
+
+  FirestoreDatabase(this._project, this._authToken);
+
+  static const apiUrl = 'https://firestore.googleapis.com/v1beta1';
+
+  String get projectUrl => '$apiUrl/projects/$_project';
+
+  String get documentsUrl => '$projectUrl/databases/(default)/documents';
+
+  String get queryUrl => '$documentsUrl:runQuery';
+
+  Map<String, String> get _headers {
+    return {
+      'Authorization': 'Bearer $_authToken',
+      'Accept': 'application/json',
+      'Content-Type': 'application/json'
+    };
+  }
+
+  Future<List> runQuery(Query query) async {
+    var body = jsonEncode(query.data);
+    var response = await _client.post(queryUrl, headers: _headers, body: body);
+    if (response.statusCode == HttpStatus.ok) {
+      return jsonDecode(response.body);
+    } else {
+      throw _error(response);
+    }
+  }
+
+  Future<Object> getDocument(String collectionName, String documentName) async {
+    var url = '$documentsUrl/$collectionName/$documentName';
+    if (_currentTransaction != null) {
+      url = '$url?transaction=${_escapedCurrentTransaction}';
+    }
+    var response = await _client.get(url, headers: _headers);
+    if (response.statusCode == HttpStatus.ok) {
+      return jsonDecode(response.body);
+    } else {
+      throw _error(response);
+    }
+  }
+
+  Future<Object> updateField(Map document, String field) async {
+    var url = '$apiUrl/${document["name"]}?updateMask.fieldPaths=$field';
+    var response =
+        await _client.patch(url, headers: _headers, body: jsonEncode(document));
+    if (response.statusCode == HttpStatus.ok) {
+      return jsonDecode(response.body);
+    } else {
+      throw _error(response);
+    }
+  }
+
+  void beginTransaction() async {
+    if (_currentTransaction != null) {
+      throw Exception('Error: nested transactions');
+    }
+    var url = '$documentsUrl:beginTransaction';
+    var body = '{"options": {}}';
+    var response = await _client.post(url, headers: _headers, body: body);
+    if (response.statusCode == HttpStatus.ok) {
+      var result = jsonDecode(response.body);
+      _currentTransaction = result['transaction'] as String;
+      if (_currentTransaction == null) {
+        throw Exception("Call returned no transaction identifier");
+      }
+    } else {
+      throw _error(response, message: 'Could not start transaction:');
+    }
+  }
+
+  Future<bool> commit([List<Write> writes]) async {
+    if (_currentTransaction == null) {
+      throw Exception('"commit" called without transaction');
+    }
+    var body = jsonEncode({
+      "writes": writes.map((write) => write.data).toList(),
+      "transaction": "$_currentTransaction"
+    });
+    var url = '$documentsUrl:commit';
+    var response = await _client.post(url, headers: _headers, body: body);
+    _currentTransaction = null;
+    if (response.statusCode == HttpStatus.conflict) {
+      // This HTTP status code corresponds to the ABORTED error code, see
+      // https://cloud.google.com/datastore/docs/concepts/errors and
+      // https://github.com/googleapis/googleapis/blob/master/google/rpc/code.proto#L137
+      return false;
+    }
+    if (response.statusCode != HttpStatus.ok) {
+      throw _error(response);
+    }
+    return true;
+  }
+
+  Exception _error(http.Response response, {String message: 'Error'}) {
+    throw Exception('$message: ${response.statusCode}: '
+        '${response.reasonPhrase}:\n${response.body}');
+  }
+
+  /// Closes the underlying HTTP client.
+  void closeClient() => _client.close();
+}
+
+abstract class Write {
+  Map get data;
+}
+
+class Update implements Write {
+  final Map data;
+  Update(List<String> updateMask, Map document, {String updateTime})
+      : data = {
+          if (updateTime != null) "currentDocument": {"updateTime": updateTime},
+          "updateMask": {"fieldPaths": updateMask},
+          "update": document
+        };
+}
+
+class Query {
+  final Map data;
+  Query(String collection, Filter filter, {int limit})
+      : data = {
+          'structuredQuery': {
+            'from': [
+              {'collectionId': collection}
+            ],
+            if (limit != null) 'limit': limit,
+            'where': filter.data,
+          }
+        };
+}
+
+class Filter {
+  final Map data;
+  Filter(this.data);
+}
+
+class FieldFilter extends Filter {
+  FieldFilter(String field, String op, String type, Object value)
+      : super({
+          'fieldFilter': {
+            'field': {'fieldPath': field},
+            'op': op,
+            'value': {'$type': value},
+          }
+        });
+}
+
+class Field {
+  final String name;
+  Field(this.name);
+  FieldFilter equals(Value value) {
+    return FieldFilter(name, 'EQUAL', value.type, value.value);
+  }
+
+  FieldFilter greaterOrEqual(Value value) {
+    return FieldFilter(name, 'GREATER_THAN_OR_EQUAL', value.type, value.value);
+  }
+
+  FieldFilter lessOrEqual(Value value) {
+    return FieldFilter(name, 'LESS_THAN_OR_EQUAL', value.type, value.value);
+  }
+
+  FieldFilter contains(Value value) {
+    return FieldFilter(name, 'ARRAY_CONTAINS', value.type, value.value);
+  }
+}
+
+class Value {
+  final String type;
+  final Object value;
+  Value.boolean(bool this.value) : type = 'booleanValue';
+  Value.string(String this.value) : type = 'stringValue';
+  Value.integer(int this.value) : type = 'integerValue';
+}
+
+class CompositeFilter extends Filter {
+  CompositeFilter(String op, List<Filter> parts)
+      : super({
+          'compositeFilter': {
+            'op': op,
+            'filters': parts.map((part) => part.data).toList(),
+          }
+        });
+}

tools/bots/update_blamelists.dart

@@ -0,0 +1,188 @@
+// Copyright (c) 2020, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+// This script is used by the bisection mechanism to update the blamelists
+// of active, non-approved failures which include the commit of the current
+// bisection build.
+
+import 'dart:io';
+
+import 'package:args/args.dart';
+
+import 'lib/src/firestore.dart';
+import 'package:test_runner/bot_results.dart';
+
+const newTest = 'new test';
+const skippedTest = 'skipped';
+
+const maxAttempts = 20;
+
+FirestoreDatabase database;
+
+class ResultRecord {
+  final data;
+
+  ResultRecord(this.data);
+
+  Map field(String name) => data['fields'][name];
+
+  int get blamelistStartIndex {
+    return int.parse(field('blamelist_start_index')['integerValue']);
+  }
+
+  void set blamelistStartIndex(int index) {
+    field('blamelist_start_index')['integerValue'] = '$index';
+  }
+
+  int get blamelistEndIndex {
+    return int.parse(field('blamelist_end_index')['integerValue']);
+  }
+
+  String get result => field('result')['stringValue'];
+
+  String get previousResult => field('previous_result')['stringValue'];
+
+  String get name => field('name')['stringValue'];
+
+  String get updateTime => data['updateTime'];
+}
+
+Query unapprovedActiveFailuresQuery(String configuration) {
+  return Query(
+      'results',
+      CompositeFilter('AND', [
+        Field('approved').equals(Value.boolean(false)),
+        // TODO(karlklose): also search for inactive failures?
+        Field('active_configurations').contains(Value.string(configuration)),
+        // TODO(karlklose): add index to check for blamelist_start_index < ?
+      ]));
+}
+
+Future<int> getCommitIndex(String commit) async {
+  try {
+    Map document = await database.getDocument('commits', commit);
+    var index = document['fields']['index'];
+    if (index['integerValue'] == null) {
+      throw Exception('Expected an integer, but got "$index"');
+    }
+    return int.parse(index['integerValue']);
+  } catch (exception) {
+    print('Could not retrieve index for commit "$commit".\n');
+    rethrow;
+  }
+}
+
+/// Compute if the record should be updated based on the outcomes in the
+/// result record and the new test result.
+bool shouldUpdateRecord(ResultRecord resultRecord, Result testResult) {
+  if (testResult == null || !testResult.matches) {
+    return false;
+  }
+  var baseline = testResult.expectation.toLowerCase();
+  if (resultRecord.previousResult.toLowerCase() != baseline) {
+    // Currently we only support the case where a bisection run improves the
+    // accuracy of a "Green" -> "Red" result record.
+    return false;
+  }
+  if (resultRecord.result.toLowerCase() == newTest ||
+      resultRecord.result.toLowerCase() == skippedTest) {
+    // Skipped tests are often configuration dependent, so it could be wrong
+    // to generalize their effect for the result record to different
+    // configurations.
+    return false;
+  }
+  return true;
+}
+
+void updateBlameLists(
+    String configuration, String commit, Map<String, Map> testResults) async {
+  int commitIndex = await getCommitIndex(commit);
+  var query = unapprovedActiveFailuresQuery(configuration);
+  bool needsRetry;
+  int attempts = 0;
+  do {
+    needsRetry = false;
+    var documents = (await database.runQuery(query))
+        .where((result) => result['document'] != null)
+        .map((result) => result['document']['name']);
+    for (var documentPath in documents) {
+      await database.beginTransaction();
+      var documentName = documentPath.split('/').last;
+      var result =
+          ResultRecord(await database.getDocument('results', documentName));
+      if (commitIndex < result.blamelistStartIndex ||
+          commitIndex >= result.blamelistEndIndex) {
+        continue;
+      }
+      String name = result.name;
+      var testResultData = testResults['$configuration:$name'];
+      var testResult =
+          testResultData != null ? Result.fromMap(testResultData) : null;
+      if (!shouldUpdateRecord(result, testResult)) {
+        continue;
+      }
+      print('Found result record: $configuration:${result.name}: '
+          '${result.previousResult} -> ${result.result} '
+          'in ${result.blamelistStartIndex}..${result.blamelistEndIndex} '
+          'to update with ${testResult.outcome} at $commitIndex.');
+      // We found a result representation for this test and configuration whose
+      // blamelist includes this results' commit but whose outcome is different
+      // then the outcome in the provided test results.
+      // This means that this commit should not be part of the result
+      // representation and we can update the lower bound of the commit range
+      // and the previous result.
+      var newStartIndex = commitIndex + 1;
+      if (newStartIndex > result.blamelistEndIndex) {
+        print('internal error: inconsistent results; skipping results entry');
+        continue;
+      }
+      result.blamelistStartIndex = newStartIndex;
+      var updateIndex = Update(['blamelist_start_index'], result.data);
+      if (!await database.commit([updateIndex])) {
+        // Commiting the change to the database had a conflict, retry.
+        needsRetry = true;
+        if (++attempts == maxAttempts) {
+          throw Exception('Exceeded maximum retry attempts ($maxAttempts).');
+        }
+        print('Transaction failed, trying again!');
+      }
+    }
+  } while (needsRetry);
+}
+
+main(List<String> arguments) async {
+  var parser = ArgParser()
+    ..addOption('auth-token',
+        abbr: 'a',
+        help: 'path to a file containing the gcloud auth token (required)')
+    ..addOption('results',
+        abbr: 'r',
+        help: 'path to a file containing the test results (required)')
+    ..addFlag('staging',
+        abbr: 's',
+        help: 'use staging database',
+        defaultsTo: true,
+        negatable: true);
+  var options = parser.parse(arguments);
+  if (options.rest.isNotEmpty ||
+      options['results'] == null ||
+      options['auth-token'] == null) {
+    print(parser.usage);
+    exit(1);
+  }
+  var results = await loadResultsMap(options['results']);
+  if (results.isEmpty) {
+    print("No test results provided, nothing to update.");
+    return;
+  }
+  // Pick an arbitrary result entry to find configuration and commit hash.
+  var firstResult = Result.fromMap(results.values.first);
+  var commit = firstResult.commitHash;
+  var configuration = firstResult.configuration;
+  var project = options['staging'] ? 'dart-ci-staging' : 'dart-ci';
+  database = FirestoreDatabase(
+      project, await readGcloudAuthToken(options['auth-token']));
+  await updateBlameLists(configuration, commit, results);
+  database.closeClient();
+}