From 778117b730ffb8580bde257388c94354f38f9699 Mon Sep 17 00:00:00 2001
From: Alex Eagle <alex@aspect.dev>
Date: Thu, 27 Jan 2022 05:23:42 -0800
Subject: [PATCH] chore: code review comments

---
 docs/Jasmine.md                                     |  3 +++
 internal/npm_install/generate_build_file.ts         | 12 ++++++++----
 internal/npm_install/index.js                       | 13 +++++++++----
 packages/jasmine/jasmine_node_test.bzl              | 11 ++++++++---
 packages/jasmine/package.json                       |  2 +-
 .../private/{jasmine_runner_test.bzl => index.bzl}  |  2 +-
 6 files changed, 30 insertions(+), 13 deletions(-)
 rename packages/jasmine/private/{jasmine_runner_test.bzl => index.bzl} (91%)

diff --git a/docs/Jasmine.md b/docs/Jasmine.md
index f47fd683b9..bf4a4b8048 100755
--- 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
index fa2448d28b..2246796b9e 100644
--- 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
index 5dca4cd2ff..b0d58be368 100644
--- 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/jasmine_node_test.bzl b/packages/jasmine/jasmine_node_test.bzl
index 1a5d7d0524..b39503e1a0 100644
--- 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
index 5a51b186f0..f538ad0b76 100644
--- 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/packages/jasmine/private/jasmine_runner_test.bzl b/packages/jasmine/private/index.bzl
similarity index 91%
rename from packages/jasmine/private/jasmine_runner_test.bzl
rename to packages/jasmine/private/index.bzl
index 22ff288292..632584ac74 100644
--- a/packages/jasmine/private/jasmine_runner_test.bzl
+++ b/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", []),