Buildbot: Only archive core dumps from unexpected crashes.

tools/testing/dart/test_progress.dart

 // Copyright (c) 2013, 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.
 
 library test_progress;
 
 import "dart:async";
 import "dart:io";
 import "dart:io" as io;
 import "dart:convert" show JSON;
@@ skipping 298 matching lines @@
   void _writeTestOutcomeRecord(Map record) {
     if (_sink == null) {
       _sink = new File(TestUtils.testOutcomeFileName())
           .openWrite(mode: FileMode.APPEND);
     }
     _sink.write("${JSON.encode(record)}\n");
   }
 }
 
 class UnexpectedCrashDumpArchiver extends EventListener {
-  final archivedBinaries = new Set<String>();
+  final archivedBinaries = <String, String>{};
 
   void done(TestCase test) {
     if (test.unexpectedOutput &&
         test.result == Expectation.CRASH &&
         test.lastCommandExecuted is ProcessCommand) {
       final name = "core.${test.lastCommandOutput.pid}";
       final file = new File(name);
       final exists = file.existsSync();
       if (exists) {
         final lastCommand = test.lastCommandExecuted as ProcessCommand;
         // We have a coredump for the process. This coredump will be archived by
         // CoreDumpArchiver (see tools/utils.py). For debugging purposes we
         // need to archive the crashed binary as well. To simplify the
         // archiving code we simply copy binaries into current folder next to
         // core dumps and name them `core.${mode}_${arch}_${binary_name}`.
         final binName = lastCommand.executable;
         final binFile = new File(binName);
         final binBaseName = new Path(binName).filename;
-        if (archivedBinaries.contains(binBaseName)) {
-          return;
+        if (!archivedBinaries.containsKey(binName) &&
+            binFile.existsSync()) {
+          final mode = test.configuration['mode'];
+          final arch = test.configuration['arch'];
+          final archived = "binary.${mode}_${arch}_${binBaseName}";
+          TestUtils.copyFile(new Path(binName), new Path(archived));
+          archivedBinaries[binName] = archived;
         }
 
-        if (binFile.existsSync()) {
-          final mode = test.configuration['mode'];
-          final arch = test.configuration['arch'];
-          TestUtils.copyFile(new Path(binName),
-              new Path("core.${mode}_${arch}_${binBaseName}"));
-          archivedBinaries.add(binBaseName);
+        if (archivedBinaries.containsKey(binName)) {
+          // We have found and copied the binary.
+          var coredumpsList;
+          try {
+            coredumpsList =
+                new File('coredumps').openSync(mode: FileMode.APPEND);

[kustermann, 2017/01/20 14:56:55]

Instead of doing this for every test, how about opening it lazily and adding a
IOSink field.

You can use the 'void allDone() {}' callback to flush the stream and close the
file.

[Vyacheslav Egorov (Google), 2017/01/20 16:44:45]

I wanted to do it for each test so that even if test.dart runner crashes we
still have at least some things logged into the file. 

It's unlikely to matter - because there should be very few crashes anyway.

+            coredumpsList.writeStringSync(
+                "${test.displayName},${name},${archivedBinaries[binName]}\n");
+          } finally {
+            if (coredumpsList != null)

[kustermann, 2017/01/20 14:56:55]

You could as well move the 'new File(...') out of the "try {" and get rid of
this "if (coredumpsList != null)".

[Vyacheslav Egorov (Google), 2017/01/20 16:44:45]

I wanted to catch and ignore exceptions from openSync, but forgot to add that
line.

+              coredumpsList.close();
+          }
         }
       }
     }
   }
 }
 
 class SummaryPrinter extends EventListener {
   final bool jsonOnly;
 
   SummaryPrinter({bool jsonOnly})
@@ skipping 342 matching lines @@
       return new VerboseProgressIndicator(startTime);
     case 'status':
       return new ProgressIndicator(startTime);
     case 'buildbot':
       return new BuildbotProgressIndicator(startTime);
     default:
       assert(false);
       return null;
   }
 }