Introduced hash valued location markers in the spec

tests/standalone/io/addlatexhash_test.dart

+// Copyright (c) 2014, 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.
+
+// testing ../../../tools/addlatexhash.dart
+
+import 'dart:io';
+import '../../../tools/addlatexhash.dart';
+
+final uri = Platform.script;
+final pathList = new List.from(uri.pathSegments)..removeLast();
+final dartRootPath = "/" + pathList.join("/") + "/../../..";

[ricow1, 2014/10/14 06:09:10]

you can do:
import 'package:path/path.dart';
and use the path package to do the manipulation, in which case it will also work
on windows (this will not)

[eernst, 2014/10/14 15:06:22]

OK, required a '--package-root=$dartroot/out/ReleaseIA32/packages' where
dartroot is a long beast.  Seems that tools/test.py provides a similar argument
(no "cannot open" raised when using test.py), so my assumption is that a test
can rely on having such a package-root.

[ricow1, 2014/10/15 08:29:05]

Yep, that is automatically passed to the vm for the tests

+
+// Check that the given ProcessResult indicates success; if so return
+// the standard output, otherwise report the failure

[ricow1, 2014/10/14 06:09:10]

comment is wrong, since this is not what we are doing here, we always return
null. 

[eernst, 2014/10/14 15:06:22]

.. or throw; but the comment should be much more true now. ;)

+checkAction(ProcessResult result, String errorMessage) {
+  var output;

[ricow1, 2014/10/14 06:09:10]

you need to set output here, otherwise it will not be set in the standard case

[eernst, 2014/10/14 15:06:22]

Forgot to resolve the case:  Is a ProcessResult even in a state where 'stdout'
etc can be read when the process fails?  The API does not mention this,
https://api.dartlang.org/apidocs/channels/dev/dartdoc-viewer/dart-io.ProcessR...,
but it seems that we can read it.  So I just do that up front. 

+  if (result.exitCode != 0) {
+    output = result.stdout;
+    print(output);
+    print(result.stderr);
+    throw errorMessage;
+  }
+  return output;
+}
+
+// testing cutMatch

[ricow1, 2014/10/14 06:09:10]

Remove comment, obvious

[eernst, 2014/10/14 15:06:22]

Done.

+
+oneTestCutMatch(line, re, expected) {
+  // Code which is commented out: useful during debugging

[ricow1, 2014/10/14 06:09:10]

we don't do commented out code, not in production code and not in tests. It is
impossible to maintain since when you change the code you will not change the
comments.

[eernst, 2014/10/14 15:06:22]

Sounds nice and clean!

But then what's the right way to preserve code that may be needed now and then
(typically: during debugging), but should not bog down deployed systems?  Always
reinvent, some kind of #ifdef, ..?

[ricow1, 2014/10/15 08:29:05]

well, normally when you need to debug the info you need is so specialized that
you might as well write it again. This will take you 30 seconds.
If you really want debugging info (which you rarely want in a test) - I would
have a top level flag printDebugInfo and condition on that in the code -
defaulting to false but easily turned on, we have that for some of our test
infrastructure code

+  // stdout.write("cutMatch: ${line} --[${re}]--> ");
+  var result = cutMatch(line, new RegExp(re).firstMatch(line));
+  // stdout.write(result + "\n");
+  return expected == result;
+}
+
+testCutMatch() {
+  oneTestCutMatch("test", "e", "tst") &&
+      oneTestCutMatch("test", "te", "st") &&
+      oneTestCutMatch("test", "st", "te") &&
+      oneTestCutMatch("test", "", "test") &&
+      oneTestCutMatch("test", "test", "")
+      ? "OK" : throw "Failed in testCutMatch";

[ricow1, 2014/10/14 06:09:10]

I would structure this differently. When this fails you have no idea which one
of the above tests failed. You can easily just do the oneTestCutMatch calls on
seperate lines and throw in oneTestCutMatch with a better error message

[eernst, 2014/10/14 15:06:22]

Done.

+}
+
+// testing sisp* functions

[ricow1, 2014/10/14 06:09:10]

same as above

[eernst, 2014/10/14 15:06:23]

Done.

+
+oneTestSisp(sispFun, line, expectation) {
+  var result = sispFun(line) == expectation;
+  // stdout.write("sispIsDart*: ${line}: ${expectation}\n");

[ricow1, 2014/10/14 06:09:10]

no commented out code

[eernst, 2014/10/14 15:06:22]

Done.

+  return result;
+}
+
+testSisp() {
+  oneTestSisp(sispIsDartBegin, "\\begin{dartCode}\n", true) &&
+      oneTestSisp(sispIsDartBegin, " \\begin{dartCode}\n", true) &&
+      oneTestSisp(sispIsDartBegin, "whatever else ..", false) &&
+      oneTestSisp(sispIsDartEnd, "\\end{dartCode}", true) &&
+      oneTestSisp(sispIsDartEnd, " \\end{dartCode}\t  \n", true) &&
+      oneTestSisp(sispIsDartEnd, "whatever else ..", false)
+      ? "OK" : throw "Failed in testSisp";

[ricow1, 2014/10/14 06:09:10]

Same comment as above, throw in the oneTestSisp function

[eernst, 2014/10/14 15:06:22]

Done.

+}
+
+testNoChange() {

[ricow1, 2014/10/14 06:09:11]

Explicitly state that this is _not_ run as part of normal testing

[eernst, 2014/10/14 15:06:22]

Done.

+  // tmp storage area
+  var tmpDir = Directory.systemTemp.createTempSync('addlatexhash_test');
+
+  // actions

[ricow1, 2014/10/14 06:09:10]

this is not actions? (also, we don't normally name sections like that, it does
not add much value)

[eernst, 2014/10/14 15:06:23]

Done.

+  final tmpDirPath = tmpDir.path;

[ricow1, 2014/10/14 06:09:10]

for all of this and the below also use the path package

[eernst, 2014/10/14 15:06:22]

Done.

+  final specDirPath = dartRootPath + "/docs/language";

[ricow1, 2014/10/14 06:09:10]

as stated above, use the path package. That said, I would normally use string
interpolation in cases like this

[eernst, 2014/10/14 15:06:22]

Done.

+  const specName = "dartLangSpec";
+  final specFileName = specName + ".tex";
+  final specPath = specDirPath + "/" + specFileName;
+  const styFileName = "dart.sty";
+  final styPath = specDirPath + "/" + styFileName;
+  const tmpName = "dartLangSpec-hash";
+  final tmpFileName = tmpName + ".tex";
+  final tmpFilePath = tmpDirPath + "/" + tmpFileName;
+  final specDviFileName = specName + ".dvi";
+  final specDviPath = specDirPath + "/" + specDviFileName;
+  final tmpDviPath = tmpDirPath + "/" + tmpName + ".dvi";
+
+  // shorthand for actions
+  runLatex(fileName,workingDirectory) => Process.runSync(
+      "latex",
+      [fileName],
+      workingDirectory: workingDirectory);
+  runAddHash() => Process.runSync(
+      "dart",
+      [dartRootPath + "/tools/addlatexhash.dart",
+        dartRootPath + "/docs/language/dartLangSpec.tex",
+        tmpDir.path + "/dartLangSpec-hash.tex"]);
+  runDvi2tty(dviFile) => Process.runSync(
+      "dvi2tty",
+      [dviFile],
+      workingDirectory: tmpDir.path);
+  chkDvi2tty(file, subject) =>
+      checkAction(runDvi2tty(file), "dvitty on $subject failed");
+
+  for (var i=0; i<5; i++) {

[ricow1, 2014/10/14 06:09:10]

space around = and <

[eernst, 2014/10/14 15:06:22]

Done.

+    checkAction(runLatex(specPath, specDirPath), "LaTeX on spec failed");
+  }
+  checkAction(runAddHash(),"addlatexhash.dart failed");
+  checkAction(Process.runSync("cp", [styPath, tmpDirPath]),

[ricow1, 2014/10/14 06:09:10]

new File('styPath').copySync('$tmpDirPath/$styFileName');

[eernst, 2014/10/14 15:06:22]

Done, except using plain styPath.

+              "copying dart.sty failed");
+  for (var i=0; i<5; i++) {

[ricow1, 2014/10/14 06:09:11]

space around = and <

[eernst, 2014/10/14 15:06:22]

Done.

+    checkAction(runLatex(tmpFileName, tmpDirPath), "LaTeX on output failed");
+  }
+  if (chkDvi2tty(specDviPath, "spec") != chkDvi2tty(tmpDviPath, "output")) {
+    throw "dvi2tty spec != dvitty output";
+  }
+}
+
+main([args]) {
+  testCutMatch();
+  testSisp();
+  // latex and dvi2tty are not installed in the standard test environment
+  if (args.length > 0 && args[0] == "local") testNoChange();
+}