Skip to content

Commit

Permalink
refactor: explicitly use runfiles helper
Browse files Browse the repository at this point in the history
This reduces our dependence on the monkey-patched require() that makes NodeJS look at the runfiles manifest when resolving modules.
Pre-factoring for changing the default of --bazel_patch_module_resolver to false
  • Loading branch information
Alex Eagle committed Dec 11, 2020
1 parent bb2a302 commit 7e0ff94
Show file tree
Hide file tree
Showing 14 changed files with 45 additions and 33 deletions.
3 changes: 2 additions & 1 deletion e2e/webapp/test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
const fs = require('fs');
const content = fs.readFileSync(require.resolve('e2e_webapp/out.min/app.js'), 'utf-8');
const runfiles = require(process.env['BAZEL_NODE_RUNFILES_HELPER']);
const content = fs.readFileSync(runfiles.resolve('e2e_webapp/out.min/app.js'), 'utf-8');
if (content.indexOf('import("./strings') < 0) {
console.error(content);
process.exitCode = 1;
Expand Down
8 changes: 4 additions & 4 deletions internal/bazel_integration_test/test_runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ const spawnSync = require('child_process').spawnSync;
const fs = require('fs');
const path = require('path');
const tmp = require('tmp');

const runfiles = require(process.env['BAZEL_NODE_RUNFILES_HELPER']);
const DEBUG = !!process.env['BAZEL_INTEGRATION_TEST_DEBUG'];
const VERBOSE_LOGS = !!process.env['VERBOSE_LOGS'];

Expand Down Expand Up @@ -189,7 +189,7 @@ if (bazelrcImportsKeys.length && isFile(bazelrcFile)) {
let bazelrcContents = fs.readFileSync(bazelrcFile, {encoding: 'utf-8'});
for (const importKey of bazelrcImportsKeys) {
const importContents =
fs.readFileSync(require.resolve(config.bazelrcImports[importKey]), {encoding: 'utf-8'});
fs.readFileSync(runfiles.resolve(config.bazelrcImports[importKey]), {encoding: 'utf-8'});
bazelrcContents = bazelrcContents.replace(importKey, importContents);
}
fs.writeFileSync(bazelrcFile, bazelrcContents);
Expand All @@ -212,7 +212,7 @@ if (config.bazelrcAppend) {
let workspaceContents = fs.readFileSync(workspaceFile, {encoding: 'utf-8'});
// replace repositories
for (const repositoryKey of Object.keys(config.repositories)) {
const archiveFile = require.resolve(config.repositories[repositoryKey]).replace(/\\/g, '/');
const archiveFile = runfiles.resolve(config.repositories[repositoryKey]).replace(/\\/g, '/');
const regex =
new RegExp(`(local_repository|http_archive|git_repository)\\(\\s*name\\s*\\=\\s*"${
repositoryKey}"[^)]+`);
Expand Down Expand Up @@ -281,7 +281,7 @@ if (isFile(packageJsonFile)) {

const isWindows = process.platform === 'win32';
const bazelBinary =
require.resolve(`${config.bazelBinaryWorkspace}/bazel${isWindows ? '.exe' : ''}`);
runfiles.resolve(`${config.bazelBinaryWorkspace}/bazel${isWindows ? '.exe' : ''}`);

if (DEBUG) {
log(`
Expand Down
5 changes: 3 additions & 2 deletions internal/check_bazel_version.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,16 @@
*/
'use strict';

const runfiles = require(process.env['BAZEL_NODE_RUNFILES_HELPER']);
const fs = require('fs');
const args = process.argv.slice(2);

const BAZEL_VERSION = args[0];

const version =
fs.readFileSync(require.resolve('build_bazel_rules_nodejs/.bazelversion'), 'utf-8').trim();
fs.readFileSync(runfiles.resolve('build_bazel_rules_nodejs/.bazelversion'), 'utf-8').trim();
const bazelci_version =
fs.readFileSync(require.resolve('build_bazel_rules_nodejs/.bazelci/presubmit.yml'), 'utf-8')
fs.readFileSync(runfiles.resolve('build_bazel_rules_nodejs/.bazelci/presubmit.yml'), 'utf-8')
.split('\n')
.find(v => v.startsWith('bazel:'));

Expand Down
6 changes: 4 additions & 2 deletions internal/npm_install/test/browserify.spec.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
const fs = require('fs');
const path = require('path');
const mainFile = 'build_bazel_rules_nodejs/third_party/npm/node_modules/browserify/index.js';
const directory = 'build_bazel_rules_nodejs/internal/npm_install/test';
const runfiles = require(process.env['BAZEL_NODE_RUNFILES_HELPER']);
const mainFile =
runfiles.resolve('build_bazel_rules_nodejs/third_party/npm/node_modules/browserify/index.js');
const directory = runfiles.resolve('build_bazel_rules_nodejs/internal/npm_install/test');

describe('our bundled, vendored browserify binary', () => {
it('should preserve licenses', () => {
Expand Down
7 changes: 4 additions & 3 deletions internal/npm_install/test/check.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
const fs = require('fs');
const path = require('path');
const unidiff = require('unidiff')
const unidiff = require('unidiff');
const runfiles = require(process.env['BAZEL_NODE_RUNFILES_HELPER']);

function check(file, updateGolden = false) {
// Strip comments from generated file for comparison to golden
// to make comparison less brittle
const actual = require.resolve(path.posix.join('fine_grained_goldens', file));
const actual = runfiles.resolve(path.posix.join('fine_grained_goldens', file));
const actualContents =
fs.readFileSync(actual, {encoding: 'utf-8'})
.replace(/\r\n/g, '\n')
Expand All @@ -18,7 +19,7 @@ function check(file, updateGolden = false) {
.replace(/[\n]+/g, '\n');

// Load the golden file for comparison
const golden = require.resolve('./golden/' + file + '.golden');
const golden = runfiles.resolvePackageRelative('./golden/' + file + '.golden');

if (updateGolden) {
// Write to golden file
Expand Down
7 changes: 3 additions & 4 deletions internal/pkg_web/test2/spec.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
const fs = require('fs');
const path = require('path');
const runfiles = require(process.env['BAZEL_NODE_RUNFILES_HELPER']);

process.chdir(path.join(process.env['TEST_SRCDIR'], 'build_bazel_rules_nodejs'));
console.error(fs.readdirSync('.'));
describe('pkg_web paths', () => {
it('should match the golden file', () => {
const output = 'build_bazel_rules_nodejs/internal/pkg_web/test2/pkg/index.html';
const golden = 'build_bazel_rules_nodejs/internal/pkg_web/test2/index_golden.html_';
const actual = fs.readFileSync(require.resolve(output), {encoding: 'utf-8'});
const expected = fs.readFileSync(require.resolve(golden), {encoding: 'utf-8'});
const actual = fs.readFileSync(runfiles.resolve(output), {encoding: 'utf-8'});
const expected = fs.readFileSync(runfiles.resolve(golden), {encoding: 'utf-8'});
// make the input hermetic by replacing the cache-buster timestamp
expect(actual.replace(/\?v=\d+/g, '?v=123').trim()).toBe(expected.trim());
});
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
const fs = require('fs');
const runfiles = require(process.env['BAZEL_NODE_RUNFILES_HELPER']);

describe('karma_web_test_suite', () => {
let config;

beforeAll(() => {
config = fs.readFileSync(
require.resolve(
runfiles.resolve(
'build_bazel_rules_nodejs/packages/concatjs/web_test/test/karma_typescript/testing_wrapped_test.conf.js'),
'utf-8');
});
Expand Down
3 changes: 2 additions & 1 deletion packages/create/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
* Simple node program to test that workspace creation works.
* We don't use a test framework here since dependencies are awkward.
*/
const pkg = 'build_bazel_rules_nodejs/packages/create/npm_package';
const runfiles = require(process.env['BAZEL_NODE_RUNFILES_HELPER']);
const pkg = runfiles.resolve('build_bazel_rules_nodejs/packages/create/npm_package');
const fs = require('fs');
const {main} = require(pkg);

Expand Down
3 changes: 2 additions & 1 deletion packages/labs/test/protobufjs/test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {TestMessage} from 'build_bazel_rules_nodejs/packages/labs/test/protobufjs/test_ts_proto';
const runfiles = require(process.env['BAZEL_NODE_RUNFILES_HELPER']!);
const {TestMessage} = require(runfiles.resolvePackageRelative('test_ts_proto.js'));

describe('protobufjs', () => {
it('should work in node', () => {
Expand Down
3 changes: 2 additions & 1 deletion packages/protractor/protractor-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
* limitations under the License.
*/

const runfiles = require(process.env['BAZEL_NODE_RUNFILES_HELPER']!);
import * as child_process from 'child_process';
import * as net from 'net';

Expand Down Expand Up @@ -98,7 +99,7 @@ export interface ServerSpec {
export async function runServer(
workspace: string, serverTarget: string, portFlag: string, serverArgs: string[],
timeout = 5000): Promise<ServerSpec> {
const serverPath = require.resolve(`${workspace}/${serverTarget}`);
const serverPath = runfiles.resolve(`${workspace}/${serverTarget}`);
const port = await findFreeTcpPort();

// Start the Bazel server binary with a random free TCP port.
Expand Down
15 changes: 8 additions & 7 deletions packages/protractor/protractor.conf.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
const runfiles = require(process.env['BAZEL_NODE_RUNFILES_HELPER']);
const path = require('path');

function log_verbose(...m) {
Expand Down Expand Up @@ -76,7 +77,7 @@ let conf = {};

// Import the user's base protractor configuration if specified
if (configPath) {
const baseConf = require(configPath);
const baseConf = require(runfiles.resolve(configPath));
if (!baseConf.config) {
throw new Error('Invalid base protractor configuration. Expected config to be exported.');
}
Expand All @@ -86,7 +87,7 @@ if (configPath) {

// Import the user's on prepare function if specified
if (onPreparePath) {
const onPrepare = require(onPreparePath);
const onPrepare = require(runfiles.resolve(onPreparePath));
if (typeof onPrepare === 'function') {
const original = conf.onPrepare;
conf.onPrepare = function() {
Expand All @@ -104,15 +105,15 @@ if (onPreparePath) {
setConf(conf, 'framework', 'jasmine2', 'is set to jasmine2');

const specs =
[TMPL_specs].map(s => require.resolve(s)).filter(s => /(\b|_)(spec|test)\.js$/.test(s));
[TMPL_specs].map(s => runfiles.resolve(s)).filter(s => /(\b|_)(spec|test)\.js$/.test(s));

setConf(conf, 'specs', specs, 'are determined by the srcs and deps attribute');

// WEB_TEST_METADATA is configured in rules_webtesting based on value
// of the browsers attribute passed to karma_web_test_suite
// We setup the protractor configuration based on the values in this object
if (process.env['WEB_TEST_METADATA']) {
const webTestMetadata = require(process.env['WEB_TEST_METADATA']);
const webTestMetadata = require(runfiles.resolve(process.env['WEB_TEST_METADATA']));
log_verbose(`WEB_TEST_METADATA: ${JSON.stringify(webTestMetadata, null, 2)}`);
if (webTestMetadata['environment'] === 'local') {
// When a local chrome or firefox browser is chosen such as
Expand All @@ -123,8 +124,8 @@ if (process.env['WEB_TEST_METADATA']) {
const webTestNamedFiles = webTestMetadata['webTestFiles'][0]['namedFiles'];
const headless = !process.env['DISPLAY'];
if (webTestNamedFiles['CHROMIUM']) {
const chromeBin = require.resolve(webTestNamedFiles['CHROMIUM']);
const chromeDriver = require.resolve(webTestNamedFiles['CHROMEDRIVER']);
const chromeBin = runfiles.resolve(webTestNamedFiles['CHROMIUM']);
const chromeDriver = runfiles.resolve(webTestNamedFiles['CHROMEDRIVER']);

// The sandbox needs to be disabled, because it causes Chrome to crash on some environments.
// See: http://chromedriver.chromium.org/help/chrome-doesn-t-start
Expand All @@ -146,7 +147,7 @@ if (process.env['WEB_TEST_METADATA']) {
// TODO(gmagolan): implement firefox support for protractor
throw new Error('Firefox not yet support by protractor_web_test_suite');

// const firefoxBin = require.resolve(webTestNamedFiles['FIREFOX'])
// const firefoxBin = runfiles.resolve(webTestNamedFiles['FIREFOX'])
// const args = [];
// if (headless) {
// args.push("--headless")
Expand Down
6 changes: 4 additions & 2 deletions packages/terser/test/directory-args.spec.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
const {directoryArgs} = require('build_bazel_rules_nodejs/packages/terser/index')
const runfiles = require(process.env['BAZEL_NODE_RUNFILES_HELPER']);
const {directoryArgs} =
require(runfiles.resolve('build_bazel_rules_nodejs/packages/terser/index.js'));
const fs = require('fs');
const path = require('path');
const tmp = require('tmp');
Expand Down Expand Up @@ -43,4 +45,4 @@ describe('directoryArgs', () => {
'--keep-fnames',
]);
});
})
})
4 changes: 2 additions & 2 deletions packages/terser/test/exec/spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ const fs = require('fs');
const cp = require('child_process');
const util = require('util');
const assert = require('assert');

const terserWrap = require.resolve('build_bazel_rules_nodejs/packages/terser/index.js');
const runfiles = require(process.env['BAZEL_NODE_RUNFILES_HELPER']);
const terserWrap = runfiles.resolve('build_bazel_rules_nodejs/packages/terser/index.js');

if (!fs.existsSync(terserWrap)) {
throw new Error(
Expand Down
5 changes: 3 additions & 2 deletions packages/terser/test/sourcemap/terser_spec.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
const fs = require('fs');
const sm = require('source-map');
const runfiles = require(process.env['BAZEL_NODE_RUNFILES_HELPER']);
const DIR = 'build_bazel_rules_nodejs/packages/terser/test/sourcemap';

describe('terser sourcemap handling', () => {
it('should produce a sourcemap output', async () => {
const file = require.resolve(DIR + '/src1.min.js.map');
const file = runfiles.resolve(DIR + '/src1.min.js.map');
const debugBuild = /\/bazel-out\/[^/\s]*-dbg\//.test(file);
const rawSourceMap = JSON.parse(fs.readFileSync(file, 'utf-8'));
await sm.SourceMapConsumer.with(rawSourceMap, null, consumer => {
Expand All @@ -18,4 +19,4 @@ describe('terser sourcemap handling', () => {
expect(pos.name).toBe('MyClass');
});
});
});
});

0 comments on commit 7e0ff94

Please sign in to comment.