Skip to content
This repository was archived by the owner on Sep 16, 2021. It is now read-only.

Windows fixes #158

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,14 @@

workspace(name = "build_bazel_rules_typescript")

# Using a pre-release snapshot to pick up a commit that makes all nodejs_binary
# programs produce source-mapped stack traces.
RULES_NODEJS_VERSION = "926349cea4cd360afcd5647ccdd09d2d2fb471aa"
# Using a pre-release snapshot to pick up a fix for puppeteer
RULES_NODEJS_VERSION = "0162fdbe8ed986c9b5d5b79e53c98385ddaf6edd"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't think you need this anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you mean switch to git_repository from the skylark rules?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nevermind, this is just a fast-forward of master


http_archive(
name = "build_bazel_rules_nodejs",
url = "https://github.com/bazelbuild/rules_nodejs/archive/%s.zip" % RULES_NODEJS_VERSION,
strip_prefix = "rules_nodejs-%s" % RULES_NODEJS_VERSION,
sha256 = "5ba3c8c209078c2e3f0c6aa4abd01a1a561f92a5bfda04e25604af5f4734d69d",
sha256 = "922327733a9ffe5961cd6edbce597d07845ea69cc753066d554137f663dedbe5",
)

load("@build_bazel_rules_nodejs//:defs.bzl", "node_repositories")
Expand Down
51 changes: 40 additions & 11 deletions examples/es5_output/es5_output_test.sh
Original file line number Diff line number Diff line change
@@ -1,40 +1,69 @@
#!/bin/bash
set -e

MANIFEST="$TEST_SRCDIR/MANIFEST"
if [ -e "$MANIFEST" ]; then
while read line; do
declare -a PARTS=($line)
if [ "${PARTS[0]}" == "build_bazel_rules_typescript/examples/some_library/library.js" ]; then
readonly LIBRARY_JS=$(cat ${PARTS[1]})
elif [ "${PARTS[0]}" == "build_bazel_rules_typescript/examples/bar.js" ]; then
readonly BAR_JS=$(cat ${PARTS[1]})
elif [ "${PARTS[0]}" == "build_bazel_rules_typescript/examples/foo.js" ]; then
readonly FOO_JS=$(cat ${PARTS[1]})
fi
done < $MANIFEST
else
readonly LIBRARY_JS=$(cat $TEST_SRCDIR/build_bazel_rules_typescript/examples/some_library/library.js)
readonly BAR_JS=$(cat $TEST_SRCDIR/build_bazel_rules_typescript/examples/bar.js)
readonly FOO_JS=$(cat $TEST_SRCDIR/build_bazel_rules_typescript/examples/foo.js)
fi

# should produce named UMD modules
readonly LIBRARY_JS=$(cat $TEST_SRCDIR/build_bazel_rules_typescript/examples/some_library/library.js)
if [[ "$LIBRARY_JS" != *"define(\"build_bazel_rules_typescript/examples/some_library/library\""* ]]; then
echo "Expected library.js to declare named module, but was"
echo "$A_JS"
echo "$LIBRARY_JS"
exit 1
fi

# should produce named UMD modules
if [[ "$BAR_JS" != *"define(\"build_bazel_rules_typescript/examples/bar\""* ]]; then
echo "Expected bar.js to declare named module, but was"
echo "$BAR_JS"
exit 1
fi

# should give a name to required modules
readonly BAR_JS=$(cat $TEST_SRCDIR/build_bazel_rules_typescript/examples/bar.js)
if [[ "$BAR_JS" != *"require(\"build_bazel_rules_typescript/examples/foo\")"* ]]; then
echo "Expected bar.js to require named module foo, but was"
echo "$BAR_JS"
exit 1
fi

# should give a name to required modules from other compilation unit
readonly FOO_JS=$(cat $TEST_SRCDIR/build_bazel_rules_typescript/examples/bar.js)
if [[ "$FOO_JS" != *"require(\"build_bazel_rules_typescript/examples/some_library/library\")"* ]]; then
if [[ "$BAR_JS" != *"require(\"build_bazel_rules_typescript/examples/some_library/library\")"* ]]; then
echo "Expected bar.js to require named module library, but was"
echo "$FOO_JS"
echo "$BAR_JS"
exit 1
fi

# should give a name to required generated modules without bazel-bin
if [[ "$FOO_JS" != *"require(\"build_bazel_rules_typescript/examples/generated_ts/foo\")"* ]]; then
echo "Expected foo.js to require generated named module foo, but was"
echo "$FOO_JS"
if [[ "$BAR_JS" != *"require(\"build_bazel_rules_typescript/examples/generated_ts/foo\")"* ]]; then
echo "Expected bar.js to require generated named module foo, but was"
echo "$BAR_JS"
exit 1
fi

# should not give a module name to external modules
if [[ "$FOO_JS" != *"require(\"typescript\")"* ]]; then
echo "Expected foo.js to require typescript by its original name, but was"
if [[ "$BAR_JS" != *"require(\"typescript\")"* ]]; then
echo "Expected bar.js to require typescript by its original name, but was"
echo "$BAR_JS"
exit 1
fi

# should produce named UMD modules
if [[ "$FOO_JS" != *"define(\"build_bazel_rules_typescript/examples/foo\""* ]]; then
echo "Expected foo.js to declare named module, but was"
echo "$FOO_JS"
exit 1
fi
17 changes: 15 additions & 2 deletions examples/es6_output/es6_output_test.sh
Original file line number Diff line number Diff line change
@@ -1,16 +1,29 @@
#!/bin/bash
set -e

MANIFEST="$TEST_SRCDIR/MANIFEST"
if [ -e "$MANIFEST" ]; then
while read line; do
declare -a PARTS=($line)
if [ "${PARTS[0]}" == "build_bazel_rules_typescript/examples/es6_output/es6_output.es6/examples/foo.js" ]; then
readonly FOO_JS=$(cat ${PARTS[1]})
elif [ "${PARTS[0]}" == "build_bazel_rules_typescript/examples/es6_output/es6_output.es6/examples/some_library/library.js" ]; then
readonly LIBRARY_JS=$(cat ${PARTS[1]})
fi
done < $MANIFEST
else
readonly FOO_JS=$(cat $TEST_SRCDIR/build_bazel_rules_typescript/examples/es6_output/es6_output.es6/examples/foo.js)
readonly LIBRARY_JS=$(cat $TEST_SRCDIR/build_bazel_rules_typescript/examples/es6_output/es6_output.es6/examples/some_library/library.js)
fi

# should not down-level ES2015 syntax, eg. `class`
readonly FOO_JS=$(cat $TEST_SRCDIR/build_bazel_rules_typescript/examples/es6_output/es6_output.es6/examples/foo.js)
if [[ "$FOO_JS" != *"class Greeter"* ]]; then
echo "Expected foo.js to contain 'class Greeter' but was"
echo "$FOO_JS"
exit 1
fi

# should not down-level ES2015 syntax, eg. `class`
readonly LIBRARY_JS=$(cat $TEST_SRCDIR/build_bazel_rules_typescript/examples/es6_output/es6_output.es6/examples/some_library/library.js)
if [[ "$LIBRARY_JS" != *"export const cool = 1;"* ]]; then
echo "Expected library.js to contain 'export const cool = 1;' but was"
echo "$LIBRARY_JS"
Expand Down
13 changes: 12 additions & 1 deletion examples/some_module/module_load_test.sh
Original file line number Diff line number Diff line change
@@ -1,7 +1,18 @@
#!/bin/bash
set -e

readonly OUT=$($TEST_SRCDIR/build_bazel_rules_typescript/examples/some_module/bin)
MANIFEST="$TEST_SRCDIR/MANIFEST"
if [ -e "$MANIFEST" ]; then
while read line; do
declare -a PARTS=($line)
if [ "${PARTS[0]}" == "build_bazel_rules_typescript/examples/some_module/bin" ]; then
readonly OUT=$(${PARTS[1]})
fi
done < $MANIFEST
else
readonly OUT=$($TEST_SRCDIR/build_bazel_rules_typescript/examples/some_module/bin)
fi

if [ "$OUT" != "hello world" ]; then
echo "Expected output 'hello world' but was $OUT"
exit 1
Expand Down
2 changes: 1 addition & 1 deletion internal/karma/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ function initConcatJs(logger, emitter, basePath) {
// Preserve all non-JS that were there in the included list.
included.push(file);
} else {
const relativePath = path.relative(basePath, file.originalPath);
const relativePath = path.relative(basePath, file.originalPath).replace(/\\/g, '/');

// Remove 'use strict'.
let content = file.content.replace(/('use strict'|"use strict");?/,
Expand Down
60 changes: 57 additions & 3 deletions internal/karma/karma.conf.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,59 @@
// Karma configuration
// GENERATED BY Bazel
const path = require('path');
const fs = require('fs');
const tmp = require('tmp');

process.env.CHROME_BIN = require('puppeteer').executablePath();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in theory the ts_web_test should allow the user to select one or more browsers,
https://github.com/bazelbuild/rules_typescript/blob/master/internal/karma/ts_web_test.bzl#L22

just to keep in mind, that anything hard-coded will need to be revisited

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll add a comment to that line explaining it a bit. Karma will just look for CHROME_BIN in the environment if it is configured to use Chrome. If another browser is selected then that value is not used.


let files = [
TMPL_files
];

const manifestFile = path.join(process.env.TEST_SRCDIR, "MANIFEST");
if (fs.existsSync(manifestFile)) {
const manifest = {};
fs.readFileSync(manifestFile, { encoding: 'utf8' })
.split('\n')
.forEach((l) => {
const m = l.split(' ');
manifest[m[0]] = m[1];
});
const manifestKeys = Object.keys(manifest);
files = files.map((f) => {
if (manifestKeys.includes(f)) {
return manifest[f];
} else {
throw new Error(`File not found in MANIFEST: ${f}`);
}
});
} else {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need to support a case where there is no MANIFEST?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as comment below. build_bazel_rules_typescript/external/ prefix is what is needed to match in the manifest and that needs to be dropped so it matches on runfiles.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can add a comment here that explains this

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cleaned up. No MANIFEST case is not needed.

files = files.map((f) => f.replace('build_bazel_rules_typescript/external/', ''));
}

let requireFiles = [
TMPL_files
].map((f) => f.replace('build_bazel_rules_typescript/external/', ''));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is rules_typescript special here? Could there be other external repos in the files?

Copy link
Contributor Author

@gregmagolan gregmagolan Mar 1, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that is to handle the two files from ts_web_test.bzl:

files_entries += [
    "build_bazel_rules_typescript/external/build_bazel_rules_typescript_karma_deps/node_modules/requirejs/require.js",
    "build_bazel_rules_typescript/external/build_bazel_rules_typescript_karma_deps/node_modules/karma-requirejs/lib/adapter.js",
  ]

I think its correct. build_bazel_rules_typescript/external/ prefix is what is needed to match in the manifest and that needs to be dropped so it matches on runfiles. in the case of the two files from ts_web_test.bz, the external workspace is build_bazel_rules_typescript_karma_deps.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh. I took a closer look at the MANIFEST file. I think this code can be improved to get rid the build_bazel_rules_typescript/external/ bit. Will try it out.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cleaned this up. No build_bazel_rules_typescript/external/ prefix is necessary.


var requireConfigContent = `
// A simplified version of Karma's requirejs.config.tpl.js for use with Karma under Bazel
(function(){
var allFiles = ` + JSON.stringify(requireFiles) + `;
var allTestFiles = [];
allFiles.forEach(function (file) {
if (/(spec|test)\\.js$/i.test(file)) {
allTestFiles.push(file.replace(/\\.js$/, ''))
}
});
require(allTestFiles, window.__karma__.start);
})();
`;

const requireConfigFile = tmp.fileSync({keep: false, postfix: '.js', dir: process.env['TEST_TMPDIR']});
console.log('Writing require config file to ', requireConfigFile.name);
fs.writeFileSync(requireConfigFile.name, requireConfigContent);
files.push(requireConfigFile.name);

module.exports = function(config) {
if (process.env['IBAZEL_NOTIFY_CHANGES'] === 'y') {
// Tell karma to only listen for ibazel messages on stdin rather than watch all the input files
Expand Down Expand Up @@ -49,8 +103,8 @@ module.exports = function(config) {

// base path that will be used to resolve all patterns (eg. files, exclude)
basePath: 'TMPL_runfiles_path',
files: [
TMPL_files
]

// files
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stray comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah. a bit redundant. there was a comment before every other property and I didn't know what else to put.

files: files,
})
}
1 change: 1 addition & 0 deletions internal/karma/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
"karma-chrome-launcher": "2.2.0",
"karma-jasmine": "1.1.1",
"karma-requirejs": "1.1.0",
"puppeteer": "1.1.0",
"requirejs": "2.3.5",
"tmp": "0.0.33"
}
Expand Down
24 changes: 18 additions & 6 deletions internal/karma/ts_web_test.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,8 @@ def _ts_web_test_impl(ctx):
# polyfilling before test libraries load.
# See https://github.com/karma-runner/karma/issues/699
files_entries += [
"build_bazel_rules_typescript_karma_deps/node_modules/requirejs/require.js",
"build_bazel_rules_typescript_karma_deps/node_modules/karma-requirejs/lib/adapter.js",
"build_bazel_rules_typescript_karma_deps/node_modules/karma/requirejs.config.tpl.js",
"build_bazel_rules_typescript/external/build_bazel_rules_typescript_karma_deps/node_modules/requirejs/require.js",
"build_bazel_rules_typescript/external/build_bazel_rules_typescript_karma_deps/node_modules/karma-requirejs/lib/adapter.js",
]
# Finally we load the user's srcs and deps
files_entries += [
Expand All @@ -81,8 +80,21 @@ def _ts_web_test_impl(ctx):
output = ctx.outputs.executable,
is_executable = True,
content = """#!/usr/bin/env bash
readonly KARMA={TMPL_karma}
readonly CONF={TMPL_conf}
MANIFEST="$TEST_SRCDIR/MANIFEST"
if [ -e "$MANIFEST" ]; then
while read line; do
declare -a PARTS=($line)
if [ "${{PARTS[0]}}" == "build_bazel_rules_typescript/{TMPL_karma}" ]; then
readonly KARMA=${{PARTS[1]}}
elif [ "${{PARTS[0]}}" == "build_bazel_rules_typescript/{TMPL_conf}" ]; then
readonly CONF=${{PARTS[1]}}
fi
done < $MANIFEST
else
readonly KARMA={TMPL_karma}
readonly CONF={TMPL_conf}
fi

export HOME=$(mktemp -d)
ARGV=( "start" $CONF )

Expand All @@ -93,7 +105,7 @@ if [ ! -z "$TEST_TMPDIR" ]; then
fi

$KARMA ${{ARGV[@]}}
""".format(TMPL_karma = ctx.executable._karma.short_path,
""".format(TMPL_karma = ctx.executable._karma.short_path.replace('..', 'external'),
TMPL_conf = conf.short_path))
return [DefaultInfo(
runfiles = ctx.runfiles(
Expand Down
Loading