Add tests for polymer-cli binary (#48)

* Add tests for polymer-cli

* Omit filenames with spaces from node_modules

* Set node_modules dir correctly for @foo/bar modules

* Collect @foo/bar modules from node_modules

* Remove only circular dependencies from a cycle, not all dependencies

* Add named-workspace-relative runfiles lookup to node_binary

Author: Zetten

.gitignore

@@ -0,0 +1 @@
+/tests/*/bazel-*

Makefile

@@ -16,7 +16,10 @@ test_typescript:
 test_webpack:
 	(cd tests/webpack && bazel test //...)
 
+test_polymer-cli:
+	(cd tests/polymer-cli && bazel test //...)
+
 test_mocha:
 	(cd tests/mocha && bazel test //...)
 
-test_all: test_helloworld test_lyrics test_express test_namespace test_typescript test_webpack test_mocha
+test_all: test_helloworld test_lyrics test_express test_namespace test_typescript test_webpack test_polymer-cli test_mocha

node/internal/node_binary.bzl

@@ -28,7 +28,7 @@ def create_launcher(ctx, output_dir, node, manifest):
     # The package path is the path 
     package_path = []
     if ctx.label.workspace_root:
-        package_path.append(ctx.label.workspace_root)
+        package_path.append(ctx.label.workspace_root.replace("external/", "", 1))
     if ctx.label.package:
         package_path.append(ctx.label.package)
 
@@ -52,25 +52,31 @@ def create_launcher(ctx, output_dir, node, manifest):
         # Namespace where node and node_modules assets have been
         # built.
         'TARGET_NAME="%s"' % ctx.attr.target,
+        # Workspace name where this rule is defined
+        'WORKSPACE_NAME="%s"' % ctx.workspace_name,
         # Based on the way the script has been invoked, $PACKAGE_PATH
         # can exist in various locations.  We need to discover this
         # and assign the value to $TARGET_PATH.
         'TARGET_PATH=""' ,
 
         'ENTRYPOINT="%s"' % entrypoint,
-        
+
         'if [[ -e "${0}_files/${NODE_EXE}" ]]; then',
         '  TARGET_PATH="${0}_files/"',
         #'  echo "Matched [standalone script] or [bazel test] context [${TARGET_PATH}]"',
         '',
-        'elif [[ -e "${0}_files/${NODE_EXE}" ]]; then',
-        '  TARGET_PATH="${0}_files/"',
-        #'  echo "Matched [standalone script] or [bazel test] context [${TARGET_PATH}]"',
-        '',
         'elif [[ -e "${0}.runfiles/__main__/${PACKAGE_PATH}/${TARGET_NAME}/${NODE_EXE}" ]]; then',
-        '  TARGET_PATH="${0}.runfiles/__main__/${PACKAGE_PATH}/${TARGET_NAME}/"', 
+        '  TARGET_PATH="${0}.runfiles/__main__/${PACKAGE_PATH}/${TARGET_NAME}/"',
         #'  echo "Matched [bazel run] context [${TARGET_PATH}]"',
         '',
+        'elif [[ -e "${0}.runfiles/${WORKSPACE_NAME}/${PACKAGE_PATH}/${TARGET_NAME}/${NODE_EXE}" ]]; then',
+        '  TARGET_PATH="${0}.runfiles/${WORKSPACE_NAME}/${PACKAGE_PATH}/${TARGET_NAME}/"',
+        #'  echo "Matched [bazel build] or [bazel run] context in a named workspace [${TARGET_PATH}]"',
+        '',
+        'elif [[ -e "${0}.runfiles/${PACKAGE_PATH}/${TARGET_NAME}/${NODE_EXE}" ]]; then',
+        '  TARGET_PATH="${0}.runfiles/${PACKAGE_PATH}/${TARGET_NAME}/"',
+        #'  echo "Matched [bazel build] context [${TARGET_PATH}]"',
+        '',
         'elif [[ -e "${PACKAGE_PATH}/${TARGET_NAME}/${NODE_EXE}" ]]; then',
         '  TARGET_PATH="${PACKAGE_PATH}/${TARGET_NAME}/"',
         #'  echo "Matched [bazel test TESTRULE] context [${TARGET_PATH}]"',

node/internal/parse_yarn_lock.js

@@ -26,19 +26,19 @@ function main() {
   //
   const entries = Object.keys(yarn.object).map(key => makeYarnEntry(key, yarn.object[key]));
 
-  // For all top-level folders in the node_modules directory that
-  // contain a package.json file...
-  const getModulesIn = p => fs.readdirSync(p)
-        .filter(f =>
-                fs.statSync(path.join(p, f)).isDirectory() &&
-                fs.existsSync(path.join(p, f, 'package.json')) &&
-                fs.statSync(path.join(p, f, 'package.json')).isFile());
-
-  // ... parse ithem 
-  const modules = getModulesIn('node_modules').map(dir => parseNodeModulePackageJson(dir));
-
-  // Iterate all the modules and merge the information from yarn into
-  // the module
+  // Scan the node_modules directory and find all top-level ('foo') or scoped (@bar/baz)
+  // modules, i.e. folders which contain a package.json file...
+  const getModulesIn = p => fs.readdirSync(p).filter(f => isPackage(p, undefined, f));
+  const findScopes = p => fs.readdirSync(p).filter(f => f.startsWith("@") && fs.statSync(path.join(p, f)).isDirectory());
+  const getModulesInScope = (p, s) => fs.readdirSync(path.join(p, s)).filter(f => isPackage(p, s, f));
+
+  // ...then parse the package.json files to collect the metadata
+  let topLevelModuleDirs = getModulesIn('node_modules');
+  let scopeModuleDirs = findScopes('node_modules').map(scope => getModulesInScope('node_modules', scope).map(m => scope+'/'+m)).reduce((a, b) => a.concat(b), []);
+  let moduleDirs = topLevelModuleDirs.concat(scopeModuleDirs);
+  const modules = moduleDirs.map(dir => parseNodeModulePackageJson(dir));
+
+  // Iterate all the modules and merge the information from yarn into the module
   modules.forEach(module => mergePackageJsonWithYarnEntry(entries, module));
 
   // Didn't realize that the nodejs module ecosystem can contain
@@ -68,6 +68,12 @@ function main() {
   print("# EOF");
 }
 
+function isPackage(p, s, f) {
+  let dir = s ? path.join(p, s, f) : path.join(p, f);
+  return fs.statSync(dir).isDirectory() &&
+    fs.existsSync(path.join(dir, 'package.json')) &&
+    fs.statSync(path.join(dir, 'package.json')).isFile()
+}
 
 /**
  * Given a list of yarn entries and a target module, find an exact
@@ -202,9 +208,10 @@ function breakCircularDependencies(modules) {
           }
         });
 
-        // Each entry in the cluster must have no other outgoing
-        // dependencies
-        entry.deps = new Set();
+        // Each entry in the cluster must have no connections to other
+        // dependencies in the cluster, or on the cluster pseudo-dep
+        pseudo.deps.forEach(circ => entry.deps.delete(circ));
+        entry.deps.delete(pseudo);
       });
 
       // Store this new pseudo-module in the modules list
@@ -278,17 +285,22 @@ function printNodeModule(module) {
   print(``);
   printJson(module);
   print(`node_module(`);
-  print(`    name = "${module.name}",`);
+  print(`    name = "${module.yarn ? module.yarn.label : module.name}",`);
 
   // SCC pseudomodule wont have 'yarn' property
   if (module.yarn) {
     const url = module.yarn.url || module.url;
     const sha1 = module.yarn.sha1;
     const executables = module.executables;
-    
+
+    print(`    module_name = "${module.name}",`);
     print(`    version = "${module.version}",`);
     print(`    package_json = "node_modules/${module.name}/package.json",`);
-    print(`    srcs = glob(["node_modules/${module.name}/**/*"], exclude = ["node_modules/${module.name}/package.json"]),`);
+    // Exclude filenames with spaces: Bazel can't cope with them (we just have to hope they aren't needed later...)
+    print(`    srcs = glob(["node_modules/${module.name}/**/*"], exclude = [
+		"node_modules/${module.name}/package.json",
+		"**/* *",
+	]),`);
     if (url) {
       print(`    url = "${url}",`);
     }
@@ -303,12 +315,12 @@ function printNodeModule(module) {
       }
       print(`    },`);
     }
-
   }
+
   if (deps && deps.size) {
     print(`    deps = [`);
     deps.forEach(dep => {
-      print(`        ":${dep.name}",`);
+      print(`        ":${dep.yarn ? dep.yarn.label : dep.name}",`);
     });
     print(`    ],`);
   }
@@ -327,7 +339,7 @@ function printNodeModuleAll(modules) {
   print(`    name = "_all_",`);
   print(`    deps = [`);
   modules.forEach(module => {
-    print(`        ":${module.name}",`);
+    print(`        ":${module.yarn ? module.yarn.label : module.name}",`);
   });
   print(`    ],`);
   print(`)`);

tests/polymer-cli/BUILD

@@ -0,0 +1,9 @@
+package(default_visibility = ["//visibility:public"])
+
+sh_test(
+    name = "polymer-cli_bin_test",
+    size = "small",
+    srcs = ["polymer-cli_bin_test.sh"],
+	data = ["@yarn_modules//:polymer-cli_polymer_bin"],
+)
+

tests/polymer-cli/README.md

@@ -0,0 +1,17 @@
+# Polymer CLI Example
+
+This folder demonstrates that the generated `BUILD` file for
+`@yarn_modules` contains a `node_binary` rule for the polymer-cli module
+executable.  It tests that the target is callable and that the help
+message is output.
+
+Polymer CLI is a fairly complex dependency that contains @-scoped dependencies
+and complex cyclic dependencies.
+
+```sh
+# Should be able to run polymer-cli directly
+$ bazel build @yarn_modules//:polymer-cli_polymer_bin -- --help
+
+# Should be able to invoke polymer-cli as standalone script 
+$ ./bazel-bin/external/yarn_modules/polymer-cli_polymer_bin --help
+```

tests/polymer-cli/WORKSPACE

@@ -0,0 +1,17 @@
+workspace(name = "org_pubref_rules_node_polymer_test")
+
+local_repository(
+    name = "org_pubref_rules_node",
+    path = "../..",
+)
+
+load("@org_pubref_rules_node//node:rules.bzl", "node_repositories", "yarn_modules")
+
+node_repositories()
+
+yarn_modules(
+    name = "yarn_modules",
+    deps = {
+        "polymer-cli": "1.5.7",
+    },
+)

tests/polymer-cli/polymer-cli_bin_test.sh

@@ -0,0 +1,7 @@
+set -e
+
+if (./external/yarn_modules/polymer-cli_polymer_bin --help &) | grep -qF '/__\/   /__\/  \/__\  The multi-tool for Polymer projects'; then
+    echo "PASS"
+else
+    exit 1
+fi