chore: code review comments

bazel-contrib · Jan 27, 2022 · 8d02c0b · 8d02c0b
1 parent 0bb1222
commit 8d02c0b
Show file tree

Hide file tree

Showing 7 changed files with 30 additions and 16 deletions.
diff --git a/docs/Jasmine.md b/docs/Jasmine.md
@@ -98,12 +98,15 @@ Defaults to `None`
 <h4 id="jasmine_node_test-jasmine">jasmine</h4>
 
 A label providing the `@bazel/jasmine` npm dependency.
+Intended for internal use only.
 
 Defaults to `None`
 
 <h4 id="jasmine_node_test-jasmine_entry_point">jasmine_entry_point</h4>
 
 A label providing the `@bazel/jasmine` entry point.
+This is a custom wrapper which adds features like sharding and ibazel support.
+Intended for internal use only.
 
 Defaults to `None`
 

diff --git a/internal/npm_install/generate_build_file.ts b/internal/npm_install/generate_build_file.ts
@@ -411,13 +411,17 @@ async function generatePackageBuildFiles(pkg: Dep) {
 
 
     const indexFile = printIndexBzl(pkg);
-    const indexFileName = hasIndexBzl ? 'private.bzl' : 'index.bzl'
     if (indexFile.length) {
-      await writeFile(path.posix.join(pkg._dir, indexFileName), indexFile);
-      buildFile += `
+      await writeFile(path.posix.join(pkg._dir, hasIndexBzl ? 'private' : '', 'index.bzl'), indexFile);
+      const buildContent = `
 # For integration testing
-exports_files(["${indexFileName}"])
+exports_files(["index.bzl"])
 `;
+      if (hasIndexBzl) {
+        await writeFile(path.posix.join(pkg._dir, 'private', 'BUILD.bazel'), buildContent);
+      } else {
+        buildFile += buildContent;
+      }
     }
 
 

diff --git a/internal/npm_install/index.js b/internal/npm_install/index.js
@@ -268,13 +268,18 @@ async function generatePackageBuildFiles(pkg) {
         }, Promise.resolve());
     }
     const indexFile = printIndexBzl(pkg);
-    const indexFileName = hasIndexBzl ? 'private.bzl' : 'index.bzl';
     if (indexFile.length) {
-        await writeFile(path.posix.join(pkg._dir, indexFileName), indexFile);
-        buildFile += `
+        await writeFile(path.posix.join(pkg._dir, hasIndexBzl ? 'private' : '', 'index.bzl'), indexFile);
+        const buildContent = `
 # For integration testing
-exports_files(["${indexFileName}"])
+exports_files(["index.bzl"])
 `;
+        if (hasIndexBzl) {
+            await writeFile(path.posix.join(pkg._dir, 'private', 'BUILD.bazel'), buildContent);
+        }
+        else {
+            buildFile += buildContent;
+        }
     }
     if (!symlinkBuildFile) {
         await writeFile(path.posix.join(pkg._dir, buildFilePath), generateBuildFileHeader(visibility) + buildFile);

diff --git a/packages/jasmine/BUILD.bazel b/packages/jasmine/BUILD.bazel
@@ -83,9 +83,6 @@ pkg_npm(
         "package.json",
     ],
     build_file_content = "",
-    substitutions = {
-        "//packages/jasmine/private:jasmine_runner_test.bzl": "//@bazel/jasmine:private.bzl",
-    },
     deps = [
         ":README.md",
         ":npm_version_check",

diff --git a/packages/jasmine/jasmine_node_test.bzl b/packages/jasmine/jasmine_node_test.bzl
@@ -19,7 +19,7 @@ than launching a test in Karma, for example.
 """
 
 load("@rules_nodejs//nodejs:providers.bzl", "JSModuleInfo")
-load("//packages/jasmine/private:jasmine_runner_test.bzl", "jasmine_runner_test")
+load("//packages/jasmine/private:index.bzl", "bazel_jasmine_runner_test")
 load("@build_bazel_rules_nodejs//internal/node:node.bzl", nodejs_test = "nodejs_test_macro")
 
 def _js_sources_impl(ctx):
@@ -67,7 +67,7 @@ def jasmine_node_test(
         tags = [],
         config_file = None,
         use_direct_specs = None,
-        # Kept for backward compatibility
+        # TODO(6.0): remove these two attributes, users should never interact with them
         jasmine = None,
         jasmine_entry_point = None,
         **kwargs):
@@ -103,7 +103,12 @@ def jasmine_node_test(
         More info: https://github.com/bazelbuild/rules_nodejs/pull/2576
 
       jasmine: A label providing the `@bazel/jasmine` npm dependency.
+        Intended for internal use only.
+
       jasmine_entry_point: A label providing the `@bazel/jasmine` entry point.
+        This is a custom wrapper which adds features like sharding and ibazel support.
+        Intended for internal use only.
+
       **kwargs: Remaining arguments are passed to the test rule
     """
     if kwargs.pop("coverage", False):
@@ -161,4 +166,4 @@ def jasmine_node_test(
             **kwargs
         )
     else:
-        jasmine_runner_test(**kwargs)
+        bazel_jasmine_runner_test(**kwargs)
diff --git a/packages/jasmine/package.json b/packages/jasmine/package.json
@@ -16,7 +16,7 @@
         "bazel"
     ],
     "bin": {
-        "jasmine_runner": "jasmine_runner.js"
+        "bazel-jasmine-runner": "jasmine_runner.js"
     },
     "main": "index.js",
     "dependencies": {

diff --git a/...s/jasmine/private/jasmine_runner_test.bzl → packages/jasmine/private/index.bzl b/...s/jasmine/private/jasmine_runner_test.bzl → packages/jasmine/private/index.bzl
@@ -5,7 +5,7 @@ from rnj sources and should not be published.
 
 load("@build_bazel_rules_nodejs//internal/node:node.bzl", nodejs_test = "nodejs_test_macro")
 
-def jasmine_runner_test(**kwargs):
+def bazel_jasmine_runner_test(**kwargs):
     nodejs_test(
         entry_point = "//packages/jasmine:jasmine_runner.js",
         data = ["//packages/jasmine"] + kwargs.pop("data", []),