Avoid flaking in message_extraction tests by doing everything in a temp directory

pkg/intl/test/message_extraction/message_extraction_test.dart

Index: pkg/intl/test/message_extraction/message_extraction_test.dart
diff --git a/pkg/intl/test/message_extraction/message_extraction_test.dart b/pkg/intl/test/message_extraction/message_extraction_test.dart
index b2de632a4a86a339390b41da856efa281f7da298..04be190392ed48058f0f4cc24154257761583c4f 100644
--- a/pkg/intl/test/message_extraction/message_extraction_test.dart
+++ b/pkg/intl/test/message_extraction/message_extraction_test.dart
@@ -10,46 +10,69 @@ import 'dart:async';
 import 'dart:convert';
 import 'package:path/path.dart' as path;
 import '../data_directory.dart';
-import 'verify_messages.dart';
-import 'sample_with_messages.dart' as sample;
 
 final dart = Platform.executable;
 
 /** The VM arguments we were given, most important package-root. */
 final vmArgs = Platform.executableArguments;
 
+var tempDir =
+    Directory.systemTemp.createTempSync('message_extraction_test').path;
+
 /**
- * Translate a file path into this test directory, regardless of the
- * working directory.
+ * Translate a relative file path into this test directory. This is
+ * applied to all the arguments of [run]. It will ignore a string that
+ * is an absolute path or begins with "--", because some of the arguments
+ * might be command-line options.
  */
-String dir([String s]) {
-  if (s != null && s.startsWith("--")) { // Don't touch command-line options.
-    return s;
-  } else {
+String inTestDir([String s]) {
+  if (s == null || s.startsWith("--") || path.isAbsolute(s)) return s;
    return path.join(intlDirectory, 'test', 'message_extraction', s);
-  }
+}
+
+/**
+ * Translate a relative file path into our temp directory. This is
+ * applied to all the arguments of [run]. It will ignore a string that
+ * is an absolute path or begins with "--", because some of the arguments
+ * might be command-line options.
+ */
+String inTempDir([String s]) {

[Emily Fortuna, 2014/03/26 17:53:07]

maybe call this something like toTempDirPath or something. with a name like
"inTempDir" I expect it to return a boolean testing if something is in a temp
directory. (same for inTestDir)

[Alan Knight, 2014/03/27 17:14:21]

Done.

+  if (s == null || s.startsWith("--") || path.isAbsolute(s)) return s;
+  return path.join(tempDir, s);
 }
 
 main() {
   test("Test round trip message extraction, translation, code generation, "
       "and printing", () {
-    deleteGeneratedFiles();
+    copyFilesToTempDirectory();
     return extractMessages(null).then((result) {
       return generateTranslationFiles(result);
     }).then((result) {
       return generateCodeFromTranslation(result);
-    }).then((_) => sample.main())
-    .then(verifyResult)
-    .whenComplete(deleteGeneratedFiles);
+    }).then((result) => runAndVerify(result));
   });
 }
 
+void copyFilesToTempDirectory() {
+  var files = [inTestDir('sample_with_messages.dart'),
+               inTestDir('part_of_sample_with_messages.dart'),
+               inTestDir('verify_messages.dart'),
+               inTestDir('run_and_verify.dart')];
+  for (var filename in files) {
+    var file = new File(filename);
+    file.copySync(path.join(tempDir, path.basename(filename)));
+  }
+}
+
 void deleteGeneratedFiles() {
-  var files = [dir('intl_messages.json'), dir('translation_fr.json'),
-      dir('translation_de_DE.json')];
-  files.map((name) => new File(name)).forEach((x) {
-    if (x.existsSync()) x.deleteSync();
-  });
+  try {
+    var dir = new Directory(tempDir);
+    dir.listSync().forEach((x) => x.deleteSync());
+    dir.deleteSync();
+  } on Error catch(e) {
+    print("Failed to delete $tempDir");
+    print("Exception:\n$e");
+  }
 }
 
 /**
@@ -68,13 +91,13 @@ Future<ProcessResult> run(ProcessResult previousResult, List<String> filenames)
     print(previousResult.stderr);
     print("exitCode=${previousResult.exitCode}");
   }
-  var filesInTheRightDirectory = filenames.map((x) => dir(x)).toList();
+  var filesInTheRightDirectory = filenames.map((x) => inTempDir(x)).toList();
   // Inject the script argument --output-dir in between the script and its
   // arguments.
   var args = []
       ..addAll(vmArgs)
       ..add(filesInTheRightDirectory.first)
-      ..addAll(["--output-dir=${dir()}"])
+      ..addAll(["--output-dir=$tempDir"])
       ..addAll(filesInTheRightDirectory.skip(1));
   var result = Process.run(dart, args, stdoutEncoding: UTF8,
       stderrEncoding: UTF8);
@@ -83,19 +106,23 @@ Future<ProcessResult> run(ProcessResult previousResult, List<String> filenames)
 
 Future<ProcessResult> extractMessages(ProcessResult previousResult) => run(
     previousResult,
-    ['extract_to_json.dart', '--suppress-warnings', 'sample_with_messages.dart',
-        'part_of_sample_with_messages.dart']);
+    [inTestDir('extract_to_json.dart'), '--suppress-warnings',
+      'sample_with_messages.dart', 'part_of_sample_with_messages.dart']);
 
 Future<ProcessResult> generateTranslationFiles(ProcessResult previousResult) =>
     run(
         previousResult,

[Emily Fortuna, 2014/03/26 17:53:07]

this feels like a curious indentation choice to me; I'd be inclined to do:

run(previousResult, 
      [inTestDir(.....)

[Alan Knight, 2014/03/27 17:14:21]

Done.

-        ['make_hardcoded_translation.dart', 'intl_messages.json']);
+        [inTestDir('make_hardcoded_translation.dart'), 'intl_messages.json']);
 
 Future<ProcessResult> generateCodeFromTranslation(ProcessResult previousResult)
     => run(
         previousResult,
-        ['generate_from_json.dart', '--generated-file-prefix=foo_',
+        [inTestDir('generate_from_json.dart'), '--generated-file-prefix=foo_',
          'sample_with_messages.dart',
              'part_of_sample_with_messages.dart', 'translation_fr.json',
              'translation_de_DE.json' ]);
 
+Future<ProcessResult> runAndVerify(ProcessResult previousResult) =>
+    run(
+        previousResult,
+        [inTempDir('run_and_verify.dart')]);