Refactor node_binary rule. (#41)

* Improved support for various execution contexts.
* Move node_modules tree creation into own rule.
* parse yarn modules: create a node_binary rules
  foreach executable target in the module.
* Redo mocha_test as an sh_test.
* Add the node_test rule.

Author: pcj

.travis.yml

@@ -8,9 +8,10 @@ os:
   - osx
 
 env:
+  - V=0.7.0
+  - V=0.6.1
   - V=0.5.4
   - V=0.5.3
-  # Not compatible under 5.3
 
 before_install:
   - OS=linux

Makefile

@@ -1,8 +1,22 @@
-test_all:
-	(cd tests/helloworld && bazel test //:helloworld_test)
-	(cd tests/lyrics && bazel test //:lyrics_test)
-	(cd tests/express && bazel test //:server_test)
-	(cd tests/namespace && bazel test //:question_test)
-	(cd tests/typescript && bazel test //:typescript_test)
-	(cd tests/mocha && bazel test //:test)
-	(cd tests/mocha && bazel test //tests:test)
+test_helloworld:
+	(cd tests/helloworld && bazel test //...)
+
+test_lyrics:
+	(cd tests/lyrics && bazel test //...)
+
+test_express:
+	(cd tests/express && bazel test //...)
+
+test_namespace:
+	(cd tests/namespace && bazel test //...)
+
+test_typescript:
+	(cd tests/typescript && bazel test //...)
+
+test_webpack:
+	(cd tests/webpack && 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

README.md

@@ -15,10 +15,11 @@
 | Rule | Description |
 | ---: | :---------- |
 | [node_repositories](#node_repositories) | Install node toolchain. |
-| [yarn_modules](#yarn_modules) | Install a set node_modules dependencies using yarn. |
-| [node_module](#node_module) | Define a node module from a set of source files and a main (or index) source file. |
-| [node_binary](#node_binary) | Build a node_modules tree and execute an entrypoint module script. |
-| [mocha_test](#mocha_test) |  Run a mocha test script. |
+| [yarn_modules](#yarn_modules) | Install a set node module dependencies using yarn. |
+| [node_module](#node_module) | Define a node module from a set of source files (having an optional main (or index) entry point). |
+| [node_binary](#node_binary) | Run a node module. |
+| [node_test](#node_test) | Run a node binary as a bazel test. |
+| [mocha_test](#mocha_test) | Run a mocha test script. |
 
 <table><tr>
 <td><img src="https://www.kernel.org/theme/images/logos/tux.png" height="48"/></td>
@@ -84,13 +85,14 @@ populate it with the necessary dependencies.
 3. Read the generated `yarn.lock` file, parse it, and write out a
    `@yarn_modules//:BUILD` file.  This file contains a `node_module`
    rule foreach entry in the `yarn.lock` file, a `node_module` rule
-   with the special name `_all_`, and an `sh_binary` rule foreach
+   with the special name `_all_`, and a `node_binary` rule foreach
    executable script in the `node_modules/.bin` folder.
 
 > Note 1: You can inspect all the targets by running `bazel query @yarn_modules//:*`.
 
 > Note 2: The workspace name `yarn_modules` is arbitrary, choose
-whatever you like *other than* `node_modules` (that one doesn't work).
+whatever you like (*other than* `node_modules` itself, that one
+doesn't work).
 
 At this point you can use these rule targets as `deps` for your
 `node_module` rules.  *Example*:
@@ -152,7 +154,7 @@ node_modules/fs-extra
 When used by other `node_module` rules, you can import the module as:
 
 ```javascript
-const myModule = require("my-module");
+const myModule = require("my_module");
 ```
 
 There are three basic ways to create a `node_module` rule:
@@ -249,6 +251,7 @@ These are only relevant if you don't explicitly name a `package.json` file.
 | optional | `string` | `url` | `None` | Url where the module tgz archive was resolved
 | optional | `string` | `sha1` | `None` | Sha1 hash of of the resolved tgz archive
 | optional | `string` | `description` | `None` | Module description
+| optional | `string_dict` | `executables` | `None` | A mapping from binary name to internal node module path.  Example `executables = { 'foo': 'bin/foo' }`.
 
 ### node_module attributes that affect the relative path of files included in the module
 
@@ -267,9 +270,7 @@ the workspace, which needs to be preserved in the generated module.
 
 ## node_binary
 
-The `node_binary` rule builds a `node_modules/` tree based on its
-`node_module` dependencies and writes a script to execute a module
-entrypoint.
+The `node_binary` rule writes a script to execute a module entrypoint.
 
 ```python
 load("@org_pubref_rules_node//node:rules.bzl", "node_binary")
@@ -299,38 +300,55 @@ the entrypoint (under the hood, it will just build a `node_module`
 becoming equivalent to the first example).
 
 
+```python
+node_binary(
+    name = "foo",
+    entrypoint = ":my_module_2",
+    executable = "baz",
+)
+```
+
+In this third example (above), we're specifying the name of the node
+module to start with (`my_module_2`) and the name of the executable
+within `my_module_2` to run (`baz`).  In this case the `node_module`
+rule definition for `my_module_2` must have a `string_dict` with an
+entry for `baz` (like `executables = { 'baz': 'bin/baz' }`.
+
 ### Output structure of files generated for a `node_binary` rule
 
-A  `node_binary` rule named `foo` will create a folder having exactly two entries:
+A `node_binary` rule named `foo` will create a folder having exactly
+two entries:
 
 1. An executable shell script named `foo`.
-1. A folder which bundles up all the needed files in `foo_bundle/`.
+1. A folder which bundles up all the needed files in `foo_files/`.
 
-Within `foo_bundle/`, there will also be exactly two entries:
+Within `foo_files/`, there will also be exactly two entries:
 
 1. The `node` executable itself.
-1. The `node_modules/` folder with all the built/copied modules.
-
-The bash shell script `foo` performs the following:
-
-`cd $(dirname $0)/foo_bundle && exec node node_modules/entrypoint`
+1. The `node_modules/` folder with all the built/copied modules
+   (including the entrypoint module).
 
 
 ### Building a deployable bundle
 
-To generate a tarred gzipped archive of the above example that you can
-ship as a single 'executable' file, invoke `$ bazel build
-:{target}_bundle.tgz`.  This is similar in intent to the java
+To generate a tarred/gzipped archive of the above example that you can
+ship as a single 'executable' self-contained package, invoke `$ bazel
+build :{target}_deploy.tar.gz`.  This is similar in intent to the java
 `{target}_deploy.jar` implicit build rule.
 
 ```sh
-$ bazel build :foo_bundle.tgz
-Target //:foo_bundle.tgz up-to-date:
+$ bazel build :foo_deploy
+Target //:foo_deploy.tar.gz up-to-date:
   bazel-bin/foo_bundle.tgz
 $ du -h bazel-bin/foo_bundle.tgz
 33M bazel-bin/foo_bundle.tgz
 ```
 
+## node_test
+
+The `node_test` rule is identical to node_binary, but sets the `test =
+True` flag such that it can be used as a bazel test.
+
 ## mocha_test
 
 Runs a mocha test identified by the start script given in `main` or
@@ -353,13 +371,9 @@ mocha_test(
     name = "test",
     main = "test.js",
 )
-
-mocha_test(
-    name = "test",
-    entrypoint = ":my_module",
-)
 ```
 
 ## Conclusion
 
-That's it!  Please refer to the various workspaces in `tests/` and the source for more detail.
+That's it!  Please refer to the various workspaces in `tests/` and the
+source for more detail.

WORKSPACE

@@ -3,3 +3,4 @@ workspace(name = "org_pubref_rules_node")
 load("//node:rules.bzl", "node_repositories")
 
 node_repositories()
+

node/BUILD

@@ -1 +1,5 @@
 package(default_visibility = ["//visibility:public"])
+
+exports_files([
+    "internal/mocha_test.sh",
+])

node/internal/mocha_test.bzl

@@ -1,104 +1,56 @@
 load("//node:internal/node_module.bzl", "node_module")
-load("//node:internal/node_binary.bzl", "copy_modules", "binary_attrs")
-
-
-def _create_launcher(ctx, output_dir, node, mocha):
-    entry_module = ctx.attr.entrypoint.node_module
-    # if package is under root
-    entrypoint = '%s_test/node_modules/%s' % (ctx.label.name, entry_module.name)
-    # if test is under inner package
-    if ctx.label.package:
-        entrypoint = '%s/%s' % (ctx.label.package, entrypoint)
-    cmd = [
-        node.short_path,
-    ] + ctx.attr.node_args + [
-        mocha.short_path,
-    ] + ctx.attr.mocha_args + [
-        entrypoint,
-    ] + ctx.attr.script_args + [
-        '$@',
-    ]
-
-    lines = [
-        '#!/usr/bin/env bash',
-        'set -e',
-        ' '.join(cmd)
-    ]
-    ctx.file_action(
-        output = ctx.outputs.executable,
-        executable = True,
-        content =  '\n'.join(lines),
+load("//node:internal/node_modules.bzl", "node_modules")
+
+def mocha_test(
+        name = None,
+        # Main test script entrypoint
+        main = None,
+        # Additional module deps for the test
+        deps = [],
+        # The mocha binary executable target
+        mocha_bin = "@mocha_modules//:mocha_bin",
+        # The script runner for the sh_test
+        script = "@org_pubref_rules_node//node:internal/mocha_test.sh",
+        # Any additional args to pass directly to mocha
+        args = [],
+        # Test size
+        size = "small",
+        # Test visibility
+        visibility = None,
+        # Remainder of args go to 'node_module'
+        **kwargs):
+
+    """Given a rule name and a main test script entrypoint file, package
+    that test script up as a module, then package that module in a
+    node_modules tree.  Run a bash script that invokes the mocha_bin
+    executable with the name of the testable entrypoint module.
+
+    """
+    
+    node_module(
+        name = name + "_module",
+        main = main,
+        visibility = visibility,
+        **kwargs
     )
 
-
-def mocha_test_impl(ctx):
-    output_dir = ctx.label.name + '_test'
-    node = ctx.executable._node
-    mocha = ctx.executable._mocha_bin
-
-    all_deps = ctx.attr.deps + [ctx.attr.entrypoint]
-    files = copy_modules(ctx, output_dir, all_deps)
-
-    _create_launcher(ctx, output_dir, node, mocha)
-
-    mocha_deps_all = ctx.attr._mocha_deps.node_module
-    transitive_mocha_files = mocha_deps_all.files.to_list()
-    for dep in mocha_deps_all.transitive_deps:
-        transitive_mocha_files += dep.files.to_list()
-
-    runfiles = [
-        node,
-        mocha,
-        ctx.outputs.executable
-    ] + transitive_mocha_files + files
-
-    return struct(
-        runfiles = ctx.runfiles(
-            files = runfiles,
-            collect_data = True,
-        ),
+    node_modules(
+        name = name + "_modules",
+        target = name + "_modules",
+        visibility = visibility,
+        deps = deps + [name + "_module"],
     )
-
-
-_mocha_test = rule(
-    mocha_test_impl,
-    attrs = binary_attrs + {
-        "_mocha_bin": attr.label(
-            default = Label("@mocha_modules//:mocha_bin"),
-            allow_files = True,
-            executable = True,
-            cfg = "host",
-        ),
-        "_mocha_deps": attr.label(
-            providers = ["node_module"],
-            default = Label("@mocha_modules//:_all_"),
-        ),
-        "mocha_args": attr.string_list(),
-    },
-    test = True,
-)
-
-
-def mocha_test(name = None, main = None, entrypoint = None, node_args = [], mocha_args = [], deps = [], visibility = None, size = "small", **kwargs):
-
-    if not entrypoint:
-        if not main:
-            fail('Either an entrypoint node_module or a main script file must be specified')
-        entrypoint = name + '_module'
-        node_module(
-            name = entrypoint,
-            main = main,
-            deps = [],
-            visibility = visibility,
-            **kwargs
-        )
-
-    _mocha_test(
+        
+    native.sh_test(
         name = name,
-        entrypoint = entrypoint,
-        deps = deps,
+        srcs = [script],
+        args = args + [
+            "{name}_modules/node_modules/{name}_module".format(name = name),
+        ],
+        data = [
+            mocha_bin,
+            name + "_modules",
+        ],
         size = size,
-        node_args = node_args,
-        mocha_args = mocha_args,
         visibility = visibility,
     )

node/internal/mocha_test.sh

@@ -0,0 +1,2 @@
+set -eu
+external/mocha_modules/mocha_bin $@