build(builtin): cleanup //internal/npm_install/test:test (#848)

Fixes local development issue on Windows where this test would fail on 2nd run due BUILD to files left on disk by the build file generator. Since Windows has no runfiles, the build file generator was running in the sources folder

.bazelignore

@@ -1,6 +1,6 @@
 node_modules
 examples/program/node_modules
-internal/npm_install/test/package/node_modules
+internal/npm_install/test/node_modules
 dist
 # Each e2e test is a nested workspace
 e2e/

.circleci/config.yml

@@ -11,7 +11,7 @@
 # If you change the `default_docker_image` version, also change the `cache_key` version
 var_1: &default_docker_image circleci/node:10.12
 var_2: &browsers_docker_image circleci/node:10.12-browsers
-var_3: &cache_key node-0.12-{{ checksum "yarn.lock" }}-{{ checksum "examples/program/yarn.lock" }}-{{ checksum "internal/npm_install/test/package/yarn.lock" }}-2
+var_3: &cache_key node-0.12-{{ checksum "yarn.lock" }}-{{ checksum "examples/program/yarn.lock" }}
 
 var_4: &init_environment
   run:
@@ -105,7 +105,6 @@ jobs:
           paths:
             - "node_modules"
             - "examples/program/node_modules"
-            - "internal/npm_install/test/package/node_modules"
 
       # Persist any changes at this point to be reused by further jobs.
       - persist_to_workspace:

.gitignore

@@ -6,7 +6,6 @@ bazel-out
 /examples/*/bazel-*
 /packages/*/bazel-*
 /internal/e2e/*/bazel-*
-/internal/npm_install/test/package/*
 node_modules
 !/internal/npm_install/test/golden/node_modules
 examples/vendored_node/node-v10.12.0-linux-x64

WORKSPACE

@@ -19,6 +19,7 @@ workspace(
         "@fine_grained_deps_yarn": ["internal/e2e/fine_grained_deps/yarn/node_modules"],
         "@fine_grained_no_bin": ["internal/e2e/fine_grained_no_bin/node_modules"],
         "@npm": ["node_modules"],
+        "@npm_install_test": ["internal/npm_install/test/node_modules"],
     },
 )
 
@@ -73,9 +74,7 @@ node_repositories(
     package_json = [
         "//:package.json",
         "@examples_program//:package.json",
-        "//internal/npm_install/test:package/package.json",
     ],
-    preserve_symlinks = True,
 )
 
 yarn_install(
@@ -147,6 +146,44 @@ yarn_install(
     yarn_lock = "//internal/e2e/fine_grained_no_bin:yarn.lock",
 )
 
+yarn_install(
+    name = "npm_install_test",
+    manual_build_file_contents = """
+filegroup(
+  name = "test_files",
+  srcs = [
+    "//:BUILD.bazel",
+    "//:install_bazel_dependencies.bzl",
+    "//:manual_build_file_contents",
+    "//:WORKSPACE",
+    "//@angular/core:BUILD.bazel",
+    "//@gregmagolan:BUILD.bazel",
+    "//@gregmagolan/test-a/bin:BUILD.bazel",
+    "//@gregmagolan/test-a:BUILD.bazel",
+    "//@gregmagolan/test-b/bin:BUILD.bazel",
+    "//@gregmagolan/test-b:BUILD.bazel",
+    "//ajv:BUILD.bazel",
+    "//jasmine/bin:BUILD.bazel",
+    "//jasmine:BUILD.bazel",
+    "//rxjs:BUILD.bazel",
+    "//unidiff/bin:BUILD.bazel",
+    "//unidiff:BUILD.bazel",
+    "//zone.js:BUILD.bazel",
+    "//node_modules/@angular/core:BUILD.bazel",
+    "//node_modules/@gregmagolan:BUILD.bazel",
+    "//node_modules/@gregmagolan/test-a:BUILD.bazel",
+    "//node_modules/@gregmagolan/test-b:BUILD.bazel",
+    "//node_modules/ajv:BUILD.bazel",
+    "//node_modules/jasmine:BUILD.bazel",
+    "//node_modules/rxjs:BUILD.bazel",
+    "//node_modules/unidiff:BUILD.bazel",
+    "//node_modules/zone.js:BUILD.bazel",
+  ],
+)""",
+    package_json = "//internal/npm_install/test:package.json",
+    yarn_lock = "//internal/npm_install/test:yarn.lock",
+)
+
 load("@bazel_toolchains//rules:rbe_repo.bzl", "rbe_autoconfig")
 
 # Creates toolchain configuration for remote execution with BuildKite CI

internal/npm_install/generate_build_file.js

@@ -702,7 +702,15 @@ function filterFiles(files, exts = []) {
       return false;
     })
   }
-  return files;
+  // Filter out internal files
+  return files.filter(file => {
+    const basenameUc = path.basename(file).toUpperCase();
+    if (basenameUc === '_WORKSPACE' || basenameUc === '_BUILD' || basenameUc === '_BUILD.BAZEL' ||
+        basenameUc === '_BAZEL_MARKER') {
+      return false;
+    }
+    return true;
+  });
 }
 
 /**

internal/npm_install/test/BUILD.bazel

@@ -1,10 +1,5 @@
 load("@build_bazel_rules_nodejs//:defs.bzl", "jasmine_node_test", "nodejs_binary")
 
-filegroup(
-    name = "package",
-    srcs = glob(["package/**"]),
-)
-
 filegroup(
     name = "goldens",
     srcs = glob(["golden/**"]),
@@ -16,10 +11,10 @@ jasmine_node_test(
     data = [
         ":check.js",
         ":goldens",
-        ":package",
         "//internal/npm_install:generate_build_file",
         "@npm//jasmine",
         "@npm//unidiff",
+        "@npm_install_test//:test_files",
     ],
 )
 
@@ -28,11 +23,10 @@ nodejs_binary(
     data = [
         ":check.js",
         ":goldens",
-        ":package",
         ":update_golden.js",
-        "//internal/npm_install:generate_build_file",
         "@npm//jasmine",
         "@npm//unidiff",
+        "@npm_install_test//:test_files",
     ],
     entry_point = ":update_golden.js",
     install_source_map_support = False,

internal/npm_install/test/check.js

@@ -1,21 +1,11 @@
-const generator = require('../generate_build_file');
 const fs = require('fs');
 const path = require('path');
 const unidiff = require('unidiff')
 
-function runGenerator() {
-  // We must change the directory to the BUILD file path
-  // so the generator is able to run
-  process.chdir(path.posix.join(path.dirname(__filename), 'package'));
-
-  // Run the BUILD file generator
-  generator.main();
-}
-
 function check(file, updateGolden = false) {
   // Strip comments from generated file for comparison to golden
   // to make comparison less brittle
-  const actual = path.posix.join(path.dirname(__filename), 'package', file);
+  const actual = require.resolve(path.posix.join('npm_install_test', file));
   const actualContents =
       fs.readFileSync(actual, {encoding: 'utf-8'})
           .replace(/\r\n/g, '\n')
@@ -56,10 +46,12 @@ Update the golden file:
 }
 
 module.exports = {
-  runGenerator,
   check,
   files: [
     'BUILD.bazel',
+    'install_bazel_dependencies.bzl',
+    'manual_build_file_contents',
+    'WORKSPACE',
     '@angular/core/BUILD.bazel',
     '@gregmagolan/BUILD.bazel',
     '@gregmagolan/test-a/bin/BUILD.bazel',

internal/npm_install/test/generate_build_file.spec.js

@@ -1,9 +1,8 @@
-const {runGenerator, check, files} = require('./check');
+const {check, files} = require('./check');
 const {printPackage} = require('../generate_build_file');
 
 describe('build file generator', () => {
   describe('integration test', () => {
-    runGenerator();
     files.forEach(file => {
       it(`should produce a BUILD file for ${file}`, () => {
         check(file);
@@ -43,7 +42,7 @@ describe('build file generator', () => {
       expect(printPackage({...pkg, _files: [], bin: {}})).not.toContain('nodejs_binary');
     });
 
-    it('bin entry is an object with an empth path', () => {
+    it('bin entry is an object with an empty path', () => {
       expect(printPackage(
                  {...pkg, _files: [], bin: {empty_string: '', _null: null, _undefined: undefined}}))
           .not.toContain('nodejs_binary');

internal/npm_install/test/golden/BUILD.bazel.golden

@@ -60,3 +60,34 @@ node_module_library(
         "//node_modules/@gregmagolan/test-b:test-b__contents",
     ],
 )
+filegroup(
+  name = "test_files",
+  srcs = [
+    "//:BUILD.bazel",
+    "//:install_bazel_dependencies.bzl",
+    "//:manual_build_file_contents",
+    "//:WORKSPACE",
+    "//@angular/core:BUILD.bazel",
+    "//@gregmagolan:BUILD.bazel",
+    "//@gregmagolan/test-a/bin:BUILD.bazel",
+    "//@gregmagolan/test-a:BUILD.bazel",
+    "//@gregmagolan/test-b/bin:BUILD.bazel",
+    "//@gregmagolan/test-b:BUILD.bazel",
+    "//ajv:BUILD.bazel",
+    "//jasmine/bin:BUILD.bazel",
+    "//jasmine:BUILD.bazel",
+    "//rxjs:BUILD.bazel",
+    "//unidiff/bin:BUILD.bazel",
+    "//unidiff:BUILD.bazel",
+    "//zone.js:BUILD.bazel",
+    "//node_modules/@angular/core:BUILD.bazel",
+    "//node_modules/@gregmagolan:BUILD.bazel",
+    "//node_modules/@gregmagolan/test-a:BUILD.bazel",
+    "//node_modules/@gregmagolan/test-b:BUILD.bazel",
+    "//node_modules/ajv:BUILD.bazel",
+    "//node_modules/jasmine:BUILD.bazel",
+    "//node_modules/rxjs:BUILD.bazel",
+    "//node_modules/unidiff:BUILD.bazel",
+    "//node_modules/zone.js:BUILD.bazel",
+  ],
+)

internal/npm_install/test/golden/WORKSPACE.golden

@@ -0,0 +1 @@
+workspace(name = "npm_install_test")

internal/npm_install/test/golden/install_bazel_dependencies.bzl.golden

@@ -0,0 +1,2 @@
+def install_bazel_dependencies():
+    """Installs all workspaces listed in bazelWorkspaces of all npm packages"""

internal/npm_install/test/golden/manual_build_file_contents.golden

@@ -0,0 +1,32 @@
+
+filegroup(
+  name = "test_files",
+  srcs = [
+    "//:BUILD.bazel",
+    "//:install_bazel_dependencies.bzl",
+    "//:manual_build_file_contents",
+    "//:WORKSPACE",
+    "//@angular/core:BUILD.bazel",
+    "//@gregmagolan:BUILD.bazel",
+    "//@gregmagolan/test-a/bin:BUILD.bazel",
+    "//@gregmagolan/test-a:BUILD.bazel",
+    "//@gregmagolan/test-b/bin:BUILD.bazel",
+    "//@gregmagolan/test-b:BUILD.bazel",
+    "//ajv:BUILD.bazel",
+    "//jasmine/bin:BUILD.bazel",
+    "//jasmine:BUILD.bazel",
+    "//rxjs:BUILD.bazel",
+    "//unidiff/bin:BUILD.bazel",
+    "//unidiff:BUILD.bazel",
+    "//zone.js:BUILD.bazel",
+    "//node_modules/@angular/core:BUILD.bazel",
+    "//node_modules/@gregmagolan:BUILD.bazel",
+    "//node_modules/@gregmagolan/test-a:BUILD.bazel",
+    "//node_modules/@gregmagolan/test-b:BUILD.bazel",
+    "//node_modules/ajv:BUILD.bazel",
+    "//node_modules/jasmine:BUILD.bazel",
+    "//node_modules/rxjs:BUILD.bazel",
+    "//node_modules/unidiff:BUILD.bazel",
+    "//node_modules/zone.js:BUILD.bazel",
+  ],
+)

internal/npm_install/test/package/package.json → internal/npm_install/test/package.json

@@ -10,8 +10,5 @@
     "rxjs": "^6.4.0",
     "unidiff": "0.0.4",
     "zone.js": "^0.9.0"
-  },
-  "scripts": {
-    "postinstall": "node delete_build_files.js"
   }
 }

internal/npm_install/test/update_golden.js

@@ -1,4 +1,3 @@
-const {runGenerator, check, files} = require('./check');
+const {check, files} = require('./check');
 
-runGenerator();
 files.forEach(file => check(file, true));