CommonJS tests are now passing.

gminor7 · Feb 18, 2016 · e9f31ee · e9f31ee
1 parent 55cc3aa
commit e9f31ee
Show file tree

Hide file tree

Showing 13 changed files with 152 additions and 20 deletions.
diff --git a/js/binary/proto_test.js b/js/binary/proto_test.js
@@ -31,6 +31,8 @@
 // Test suite is written using Jasmine -- see http://jasmine.github.io/
 
 goog.require('goog.testing.asserts');
+
+// CommonJS-LoadFromFile: testbinary_pb
 goog.require('proto.jspb.test.ExtendsWithMessage');
 goog.require('proto.jspb.test.ForeignEnum');
 goog.require('proto.jspb.test.ForeignMessage');

diff --git a/js/binary/reader_test.js b/js/binary/reader_test.js
@@ -42,6 +42,8 @@
  */
 
 goog.require('goog.testing.asserts');
+
+// CommonJS-LoadFromFile: google_protobuf
 goog.require('jspb.BinaryConstants');
 goog.require('jspb.BinaryDecoder');
 goog.require('jspb.BinaryReader');

diff --git a/js/binary/utils_test.js b/js/binary/utils_test.js
@@ -38,6 +38,8 @@
 
 goog.require('goog.crypt.base64');
 goog.require('goog.testing.asserts');
+
+// CommonJS-LoadFromFile: google_protobuf
 goog.require('jspb.BinaryConstants');
 goog.require('jspb.BinaryWriter');
 goog.require('jspb.utils');

diff --git a/js/commonjs_export.js b/js/commonjs_export.js
@@ -2,9 +2,10 @@
  * @fileoverview Export symbols needed by generated code in CommonJS style.
  */
 
-exports = {
-  Message: jspb.Message,
-  BinaryReader: jspb.BinaryReader,
-  BinaryWriter: jspb.BinaryWriter,
-  ExtensionFieldInfo: jspb.ExtensionFieldInfo,
-};
+exports.Message = jspb.Message;
+exports.BinaryReader = jspb.BinaryReader;
+exports.BinaryWriter = jspb.BinaryWriter;
+exports.ExtensionFieldInfo = jspb.ExtensionFieldInfo;
+
+exports.exportSymbol = goog.exportSymbol;
+exports.inherits = goog.inherits;
diff --git a/js/commonjs_export_asserts.js b/js/commonjs_export_asserts.js
@@ -0,0 +1,27 @@
+/**
+ * @fileoverview Description of this file.
+ */
+
+goog.require('goog.testing.asserts');
+
+var global = Function('return this')();
+
+// The Google Closure assert functions start with assert, eg.
+//   assertThrows
+//   assertNotThrows
+//   assertTrue
+//   ...
+//
+// The one exception is the "fail" function.
+function shouldExport(str) {
+  return str.lastIndexOf('assert') === 0 || str == 'fail';
+}
+
+for (var key in global) {
+  if ((typeof key == "string") && global.hasOwnProperty(key) &&
+      shouldExport(key)) {
+    exports[key] = global[key];
+  }
+}
+
+exports.COMPILED = COMPILED
diff --git a/js/debug_test.js b/js/debug_test.js
@@ -31,13 +31,16 @@
 goog.setTestOnly();
 
 goog.require('goog.testing.asserts');
+
+// CommonJS-LoadFromFile: google-protobuf
 goog.require('jspb.debug');
+
+// CommonJS-LoadFromFile: test_pb
 goog.require('proto.jspb.test.HasExtensions');
 goog.require('proto.jspb.test.IsExtension');
 goog.require('proto.jspb.test.Simple1');
 
 
-
 describe('debugTest', function() {
   it('testSimple1', function() {
     if (COMPILED) {

diff --git a/js/gulpfile.js b/js/gulpfile.js
@@ -1,5 +1,6 @@
 var gulp = require('gulp');
 var exec = require('child_process').exec;
+var glob = require('glob');
 
 gulp.task('genproto_closure', function (cb) {
   exec('../src/protoc --js_out=library=testproto_libs,binary:. -I ../src -I . *.proto ../src/google/protobuf/descriptor.proto',
@@ -22,7 +23,38 @@ gulp.task('genproto_commonjs', function (cb) {
 gulp.task('dist', function (cb) {
   // TODO(haberman): minify this more aggressively.
   // Will require proper externs/exports.
-  exec('./node_modules/google-closure-library/closure/bin/calcdeps.py -i message.js -i binary/reader.js -i binary/writer.js -p . -p node_modules/google-closure-library/closure -o compiled --compiler_jar node_modules/google-closure-compiler/compiler.jar > google-protobuf.js',
+  exec('./node_modules/google-closure-library/closure/bin/calcdeps.py -i message.js -i binary/reader.js -i binary/writer.js -i commonjs_export.js -p . -p node_modules/google-closure-library/closure -o compiled --compiler_jar node_modules/google-closure-compiler/compiler.jar > google-protobuf.js',
+       function (err, stdout, stderr) {
+    console.log(stdout);
+    console.log(stderr);
+    cb(err);
+  });
+});
+
+gulp.task('commonjs_asserts', function (cb) {
+  exec('mkdir -p commonjs_out && ./node_modules/google-closure-library/closure/bin/calcdeps.py -i commonjs_export_asserts.js -p . -p node_modules/google-closure-library/closure -o compiled --compiler_jar node_modules/google-closure-compiler/compiler.jar > commonjs_out/closure_asserts_commonjs.js',
+       function (err, stdout, stderr) {
+    console.log(stdout);
+    console.log(stderr);
+    cb(err);
+  });
+});
+
+gulp.task('make_commonjs_out', ['dist', 'genproto_commonjs', 'commonjs_asserts'], function (cb) {
+  // TODO(haberman): minify this more aggressively.
+  // Will require proper externs/exports.
+  var cmd = "mkdir -p commonjs_out/binary && ";
+  function addTestFile(file) {
+    cmd += 'nodejs rewrite_tests_for_commonjs.js < ' + file +
+           ' > commonjs_out/' + file + '&& ';
+  }
+
+  glob.sync('*_test.js').forEach(addTestFile);
+  glob.sync('binary/*_test.js').forEach(addTestFile);
+
+  exec(cmd +
+       'cp jasmine_commonjs.json commonjs_out/jasmine.json && ' +
+       'cp google-protobuf.js commonjs_out',
        function (err, stdout, stderr) {
     console.log(stdout);
     console.log(stderr);
@@ -48,8 +80,8 @@ gulp.task('test_closure', ['genproto_closure', 'deps'], function (cb) {
   });
 });
 
-gulp.task('test_commonjs', ['genproto_commonjs', 'dist'], function (cb) {
-  exec('JASMINE_CONFIG_PATH=jasmine.json cp jasmine_commonjs.json commonjs_out/jasmine.json && cd commonjs_out && ../node_modules/.bin/jasmine',
+gulp.task('test_commonjs', ['make_commonjs_out'], function (cb) {
+  exec('cd commonjs_out && JASMINE_CONFIG_PATH=jasmine.json NODE_PATH=. ../node_modules/.bin/jasmine',
        function (err, stdout, stderr) {
     console.log(stdout);
     console.log(stderr);

diff --git a/js/jasmine_commonjs.json b/js/jasmine_commonjs.json
@@ -1,11 +1,9 @@
 {
     "spec_dir": "",
     "spec_files": [
-        "*_test.js"
+        "*_test.js",
+        "binary/proto_test.js"
     ],
     "helpers": [
-        "node_modules/google-closure-library/closure/goog/bootstrap/nodejs.js",
-        "node_loader.js",
-        "deps.js"
     ]
 }
diff --git a/js/message_test.js b/js/message_test.js
@@ -34,17 +34,25 @@ goog.setTestOnly();
 
 goog.require('goog.json');
 goog.require('goog.testing.asserts');
+
+// CommonJS-LoadFromFile: google-protobuf
 goog.require('jspb.Message');
+
+// CommonJS-LoadFromFile: test5_pb
 goog.require('proto.jspb.exttest.beta.floatingStrField');
+
+// CommonJS-LoadFromFile: test3_pb
 goog.require('proto.jspb.exttest.floatingMsgField');
+
+// CommonJS-LoadFromFile: test4_pb
 goog.require('proto.jspb.exttest.floatingMsgFieldTwo');
+
+// CommonJS-LoadFromFile: test_pb
 goog.require('proto.jspb.test.CloneExtension');
 goog.require('proto.jspb.test.Complex');
 goog.require('proto.jspb.test.DefaultValues');
 goog.require('proto.jspb.test.Empty');
 goog.require('proto.jspb.test.EnumContainer');
-goog.require('proto.jspb.test.ExtensionMessage');
-goog.require('proto.jspb.test.floatingMsgField');
 goog.require('proto.jspb.test.floatingStrField');
 goog.require('proto.jspb.test.HasExtensions');
 goog.require('proto.jspb.test.IndirectExtension');
@@ -56,13 +64,16 @@ goog.require('proto.jspb.test.Simple1');
 goog.require('proto.jspb.test.Simple2');
 goog.require('proto.jspb.test.SpecialCases');
 goog.require('proto.jspb.test.TestClone');
-goog.require('proto.jspb.test.TestExtensionsMessage');
 goog.require('proto.jspb.test.TestGroup');
 goog.require('proto.jspb.test.TestGroup1');
 goog.require('proto.jspb.test.TestMessageWithOneof');
 goog.require('proto.jspb.test.TestReservedNames');
 goog.require('proto.jspb.test.TestReservedNamesExtension');
 
+// CommonJS-LoadFromFile: test2_pb
+goog.require('proto.jspb.test.ExtensionMessage');
+goog.require('proto.jspb.test.TestExtensionsMessage');
+goog.require('proto.jspb.test.floatingMsgField');
 
 
 

diff --git a/js/package.json b/js/package.json
@@ -9,10 +9,11 @@
     "jasmine": "~2.4.1"
   },
   "devDependencies": {
-    "google-closure-compiler": "~20151216.2.0"
+    "google-closure-compiler": "~20151216.2.0",
+    "glob": "~6.0.4"
   },
   "scripts": {
-    "test": "./node_modules/gulp/bin/gulp.js test"
+    "test": "./node_modules/gulp/bin/gulp.js test_closure"
   },
   "repository": {
     "type": "git",

diff --git a/js/proto3_test.js b/js/proto3_test.js
@@ -29,7 +29,11 @@
 // OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 goog.require('goog.testing.asserts');
+
+// CommonJS-LoadFromFile: testbinary_pb
 goog.require('proto.jspb.test.ForeignMessage');
+
+// CommonJS-LoadFromFile: proto3_test_pb
 goog.require('proto.jspb.test.Proto3Enum');
 goog.require('proto.jspb.test.TestProto3');
 

diff --git a/js/rewrite_tests_for_commonjs.js b/js/rewrite_tests_for_commonjs.js
@@ -0,0 +1,45 @@
+/**
+ * @fileoverview Description of this file.
+ */
+
+var lineReader = require('readline').createInterface({
+  input: process.stdin,
+  output: process.stdout
+});
+
+var module = null;
+lineReader.on('line', function(line) {
+  var is_require = line.match(/goog\.require\('([^']*\.)([^'.]+)'\)/);
+  var is_loadfromfile = line.match(/CommonJS-LoadFromFile: (.*)/);
+  var is_settestonly = line.match(/goog.setTestOnly()/);
+  if (is_settestonly) {
+    // Remove this line.
+  } else if (is_require) {
+    if (module) {  // Skip goog.require() lines before the first directive.
+      var pkg = is_require[1];
+      var sym = is_require[2];
+      console.log("google_protobuf.exportSymbol('" + pkg + sym + "', " + module + "." + sym + ', global);');
+    }
+  } else if (is_loadfromfile) {
+    if (!module) {
+      console.log("var asserts = require('closure_asserts_commonjs');");
+      console.log("var global = Function('return this')();");
+      console.log("");
+      console.log("// Bring asserts into the global namespace.");
+      console.log("for (var key in asserts) {");
+      console.log("  if (asserts.hasOwnProperty(key)) {");
+      console.log("    global[key] = asserts[key];");
+      console.log("  }");
+      console.log("}");
+      console.log("");
+      console.log("var google_protobuf = require('google-protobuf');");
+    }
+    module = is_loadfromfile[1].replace("-", "_");
+
+    if (module != "google_protobuf") {  // We unconditionally require this in the header.
+      console.log("var " + module + " = require('" + is_loadfromfile[1] + "');");
+    }
+  } else {
+    console.log(line);
+  }
+});
diff --git a/src/google/protobuf/compiler/js/js_generator.cc b/src/google/protobuf/compiler/js/js_generator.cc
@@ -2516,6 +2516,10 @@ void Generator::GenerateFile(const GeneratorOptions& options,
 
   // Generate "require" statements.
   if (options.import_style == GeneratorOptions::IMPORT_COMMONJS) {
+    printer->Print("var jspb = require('google-protobuf');\n");
+    printer->Print("var goog = jspb;\n");
+    printer->Print("var global = Function('return this')();\n\n");
+
     for (int i = 0; i < file->dependency_count(); i++) {
       const std::string& name = file->dependency(i)->name();
       printer->Print(
@@ -2537,7 +2541,7 @@ void Generator::GenerateFile(const GeneratorOptions& options,
   //FindProvidesForFields(options, printer, extensions, &provided);
   for (std::set<string>::iterator it = provided.begin();
        it != provided.end(); ++it) {
-    printer->Print("goog.exportSymbol('$name$', null, this);\n",
+    printer->Print("goog.exportSymbol('$name$', null, global);\n",
                    "name", *it);
   }