Move shared testing code into test-util.js

Move shared testing code into test-util.js

BUG=None
TEST=All tests still pass

Committed: https://code.google.com/p/traceur-compiler/source/detail?r=c429d08

[usrbincc, 2012-11-11 13:26:51]

I agree that no matter what, a lot of these functions are better broken
out into a utility library, but if the goal is to be able to use those
functions to share code with an output-checking script, see the next
section.

----

I don't know why I didn't notice it before, but you can actually easily add
output checking to the current structure:

#--cut--

git checkout 4508897258b4781e   # recent HEAD for branch master
git checkout -b issue6772044-test-output

cat > test-output.diff <<"END"
diff --git a/test/feature/out.js b/test/feature/out.js
new file mode 100644
index 0000000..aad1052
--- /dev/null
+++ b/test/feature/out.js
@@ -0,0 +1,7 @@
+// Test output.
+/*
+--cut--
+var a;
+--cut--
+*/
+var a;
diff --git a/test/testfeatures.js b/test/testfeatures.js
index 7a0a31a..87bf786 100644
--- a/test/testfeatures.js
+++ b/test/testfeatures.js
@@ -120,6 +120,7 @@ function testScript(filePath) {
   var skip = false;
   var shouldCompile = true;
   var expectedErrors = [];
+  var testOutput = false;
   forEachPrologLine(source, function(line) {
     var m;
     if (line.indexOf('// Only in browser.') === 0) {
@@ -132,6 +133,8 @@ function testScript(filePath) {
       traceur.options.fromString(m[1]);
     } else if ((m = /\/\/ Error:\s*(.+)/.exec(line))) {
       expectedErrors.push(m[1]);
+    } else if (line.indexOf('// Test output.') === 0) {
+      testOutput = true;
     }
   });

@@ -175,6 +178,24 @@ function testScript(filePath) {
       return false;
     }

+    if (testOutput) {
+      var f = function(k, v) {
+        return k === 'location' ? undefined : v;
+      }
+      var SourceFile = traceur.syntax.SourceFile;
+      var compileFile = traceur.codegeneration.Compiler.compileFile;
+
+      var s1 = new SourceFile(filePath, source.split('--cut--')[1]);
+      var t1 = compileFile(reporter, s1, filePath);
+      var o1 = JSON.stringify(t1, f, 2);
+      var o2 = JSON.stringify(tree, f, 2);
+      if (o1 === o2)
+        return true;
+
+      failScript(filePath, 'Output mismatch.');
+      return false;
+    }
+
     var TreeWriter = traceur.outputgeneration.TreeWriter;
     var javascript = TreeWriter.write(tree, false);
END

git apply test-output.diff
node test/testfeatures.js

#--cut--

[usrbincc, 2012-11-11 13:28:55]

I got caught again by the publish + mail drafts UI trap.

https://codereview.appspot.com/6772044/diff/1/test/testfeatures.js
File test/testfeatures.js (left):

https://codereview.appspot.com/6772044/diff/1/test/testfeatures.js#oldcode299
test/testfeatures.js:299: }
Do you plan to eventually do something like

  var asserts = testUtil.asserts;

to allow the for loop above to work unchanged?

[arv, 2012-11-11 16:59:10]

This patch was a step in the direction of moving more tests to run in node as
well as in browsers.

https://codereview.appspot.com/6772044/diff/1/test/testfeatures.js
File test/testfeatures.js (left):

https://codereview.appspot.com/6772044/diff/1/test/testfeatures.js#oldcode299
test/testfeatures.js:299: }
On 2012/11/11 13:28:55, usrbincc wrote:
> Do you plan to eventually do something like
> 
>   var asserts = testUtil.asserts;
> 
> to allow the for loop above to work unchanged?

At some point I would like to get rid of the dependency on Closure Library for
testing. It is too large for our need. Once we have something else we can
revisit this.

[usrbincc, 2012-11-11 22:18:38]

Sorry about that. I actually entirely skipped by the feature_test.html
diff, because I hardly use the html tests. It makes much more sense now.

I think it should work fine. It looks like the only things that you need
are the asserts, a 'fail' function, and something to run all the tests.

  // Simplest possible implementation
  function fail(msg) {
    console.error(msg);
  }
  Object.keys(window).filter(function(k) {
    return k.match(/^test/);
  }).forEach(function(k) {
    window[k]();
  });

As long as the new tests don't have to be as pretty as the old ones, it
should be fairly straightforward for the individual tests. It's the
test/alltests.html code that I'm not too sure about.

  I don't do much advanced DOM, but I think just loading the files in
  the testPaths array serially in an iframe and using exceptions or
  maybe passing a callback (to pass test results back to the main page)
  into the iframe (don't know if it's possible) would work. Or maybe I
  should just look at how closure does it. I'm trying to avoid that for
  now, though. Closure, as you said (in a different context) is too
  large.

[arv, 2012-11-12 15:03:44]

On 2012/11/11 22:18:38, usrbincc wrote:
> Sorry about that. I actually entirely skipped by the feature_test.html
> diff, because I hardly use the html tests. It makes much more sense now.
> 
> I think it should work fine. It looks like the only things that you need
> are the asserts, a 'fail' function, and something to run all the tests.
> 
>   // Simplest possible implementation
>   function fail(msg) {
>     console.error(msg);
>   }
>   Object.keys(window).filter(function(k) {
>     return k.match(/^test/);
>   }).forEach(function(k) {
>     window[k]();
>   });
> 
> As long as the new tests don't have to be as pretty as the old ones, it
> should be fairly straightforward for the individual tests. It's the
> test/alltests.html code that I'm not too sure about.
> 
>   I don't do much advanced DOM, but I think just loading the files in
>   the testPaths array serially in an iframe and using exceptions or
>   maybe passing a callback (to pass test results back to the main page)
>   into the iframe (don't know if it's possible) would work. Or maybe I
>   should just look at how closure does it. I'm trying to avoid that for
>   now, though. Closure, as you said (in a different context) is too
>   large.

This is off topic.

The Closure tests does a lot more than this but I agree that at the core this is
the important part.

Lets focus on this patch. Is it good enough or are there things that needs to be
done before committed?

[arv, 2012-11-12 15:11:27]

https://codereview.appspot.com/6772044/diff/1/test/testfeatures.js
File test/testfeatures.js (left):

https://codereview.appspot.com/6772044/diff/1/test/testfeatures.js#oldcode299
test/testfeatures.js:299: }
On 2012/11/11 13:28:55, usrbincc wrote:
> Do you plan to eventually do something like
> 
>   var asserts = testUtil.asserts;
> 
> to allow the for loop above to work unchanged?

The Closure asserts are a lot richer so I don't want to replace those yet.

[usrbincc, 2012-11-12 16:13:29]

Right, focus, focus.

The architectural change LGTM. It's code duplication, and should be
eliminated.

It looks like you want to think on it a little before making the break
with Closure Library, so I'll wait for a later patch to get into that.

[usrbincc, 2012-11-12 16:13:39]

Right, focus, focus.

The architectural change LGTM. It's code duplication, and should be
eliminated.

It looks like you want to think on it a little before making the break
with Closure Library, so I'll wait for a later patch to get into that.

[slightlylate, 2012-11-12 16:48:06]

LGTM