Skip to content

Commit

Permalink
Replace babel with esbuild for tests and for webpack (#6527)
Browse files Browse the repository at this point in the history
* try esbuild again

* fix snapshot look up and missing BABEL_ENV var

* run on all test files

* use newer target for esbuild compilation of tests

* also use browserlist for esbuild; clean up

* Revert "use newer target for esbuild compilation of tests" as this seems
to cause the CI fail to fail

This reverts commit 63afd63.

* remove nyc

* remove obsolete comment

* Update webpack.config.js

* rename babel_env to is_testing

* explain target=node12

* add example comments to complex bash command in test.sh
  • Loading branch information
philippotto authored Jan 5, 2023
1 parent 6ab1bcd commit 5995054
Show file tree
Hide file tree
Showing 14 changed files with 568 additions and 1,652 deletions.
67 changes: 0 additions & 67 deletions .babelrc

This file was deleted.

2 changes: 1 addition & 1 deletion frontend/javascripts/libs/error_handling.ts
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ class ErrorHandling {
optParams: Record<string, any> = {},
severity: "error" | "warning" = "error",
) {
if (process.env.BABEL_ENV === "test") {
if (process.env.IS_TESTING) {
return;
}

Expand Down
2 changes: 1 addition & 1 deletion frontend/javascripts/oxalis/api/api_loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class Api {
* });
*/
apiReady(version: number = latestVersion): Promise<Record<string, any>> {
if (process.env.BABEL_ENV !== "test") {
if (!process.env.IS_TESTING) {
if (version !== latestVersion) {
console.warn(`
Attention! You requested api version: ${version} which is
Expand Down
4 changes: 2 additions & 2 deletions frontend/javascripts/oxalis/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -311,8 +311,8 @@ const Constants = {
// Maximum of how many buckets will be held in RAM (per layer)
MAXIMUM_BUCKET_COUNT_PER_LAYER: 5000,
FLOOD_FILL_EXTENTS: {
_2D: (process.env.BABEL_ENV === "test" ? [512, 512, 1] : [768, 768, 1]) as Vector3,
_3D: (process.env.BABEL_ENV === "test" ? [64, 64, 32] : [96, 96, 96]) as Vector3,
_2D: (process.env.IS_TESTING ? [512, 512, 1] : [768, 768, 1]) as Vector3,
_3D: (process.env.IS_TESTING ? [64, 64, 32] : [96, 96, 96]) as Vector3,
},
};
export default Constants;
Expand Down
9 changes: 4 additions & 5 deletions frontend/javascripts/oxalis/controller.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,7 @@ type State = {
class Controller extends React.PureComponent<PropsWithRouter, State> {
// @ts-expect-error ts-migrate(2564) FIXME: Property 'keyboardNoLoop' has no initializer and i... Remove this comment to see the full error message
keyboardNoLoop: InputKeyboardNoLoop;
// @ts-expect-error ts-migrate(2564) FIXME: Property 'isMounted' has no initializer and is not... Remove this comment to see the full error message
isMounted: boolean;
_isMounted: boolean = false;
state: State = {
gotUnhandledError: false,
organizationToSwitchTo: null,
Expand All @@ -80,7 +79,7 @@ class Controller extends React.PureComponent<PropsWithRouter, State> {
// The annotation view should be rendered without the special mobile-friendly
// viewport meta tag.
Utils.disableViewportMetatag();
this.isMounted = true;
this._isMounted = true;
Store.dispatch(setIsInAnnotationViewAction(true));
UrlManager.initialize();

Expand All @@ -92,7 +91,7 @@ class Controller extends React.PureComponent<PropsWithRouter, State> {
}

componentWillUnmount() {
this.isMounted = false;
this._isMounted = false;
Store.dispatch(setIsInAnnotationViewAction(false));
}

Expand Down Expand Up @@ -155,7 +154,7 @@ class Controller extends React.PureComponent<PropsWithRouter, State> {
window.onbeforeunload = null; // clear the event handler otherwise it would be called twice. Once from history.block once from the beforeunload event

setTimeout(() => {
if (!this.isMounted) {
if (!this._isMounted) {
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -567,7 +567,7 @@ export class DataBucket {

if (data.length !== channelCount * Constants.BUCKET_SIZE) {
const debugInfo = // Disable this conditional if you need verbose output here.
process.env.BABEL_ENV === "test"
process.env.IS_TESTING
? " (<omitted>)"
: {
arrayBuffer,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ export function getSupportedTextureSpecs(): GpuSpecs {
const supportedTextureSize = gl.getParameter(gl.MAX_TEXTURE_SIZE);
const maxTextureImageUnits = gl.getParameter(gl.MAX_TEXTURE_IMAGE_UNITS);

if (process.env.BABEL_ENV !== "test") {
if (!process.env.IS_TESTING) {
console.log("maxTextureImageUnits", maxTextureImageUnits);
}

Expand Down Expand Up @@ -291,7 +291,7 @@ export function computeDataTexturesSetup<
hasSegmentation,
);

if (process.env.BABEL_ENV !== "test") {
if (!process.env.IS_TESTING) {
console.log("maximumLayerCountToRender", maximumLayerCountToRender);
}

Expand Down
2 changes: 1 addition & 1 deletion frontend/javascripts/oxalis/model/sagas/root_saga.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ function* restartableSaga(): Saga<void> {
rootSagaCrashed = true;
console.error("The sagas crashed because of the following error:", err);

if (process.env.BABEL_ENV !== "test") {
if (!process.env.IS_TESTING) {
// @ts-ignore
ErrorHandling.notify(err, {});
// Hide potentially old error highlighting which mentions a retry mechanism.
Expand Down
2 changes: 1 addition & 1 deletion frontend/javascripts/oxalis/model_initialization.ts
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,7 @@ function initializeDataLayerInstances(gpuFactor: number | null | undefined): {
maximumTextureCountForLayer,
} = validateSpecsForLayers(dataset, requiredBucketCapacity);

if (process.env.BABEL_ENV !== "test") {
if (!process.env.IS_TESTING) {
console.log("Supporting", smallestCommonBucketCapacity, "buckets");
}

Expand Down
7 changes: 5 additions & 2 deletions frontend/javascripts/test/helpers/apiHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,9 +100,12 @@ const { setSlowCompression } = mockRequire.reRequire(
);
// Avoid node caching and make sure all mockRequires are applied
const UrlManager = mockRequire.reRequire("oxalis/controller/url_manager").default;
const wkstoreAdapter = mockRequire.reRequire("oxalis/model/bucket_data_handling/wkstore_adapter");
let wkstoreAdapter = mockRequire.reRequire("oxalis/model/bucket_data_handling/wkstore_adapter");

wkstoreAdapter.requestFromStore = () => new Uint8Array();
wkstoreAdapter = {
...wkstoreAdapter,
requestFromStore: () => new Uint8Array(),
};

mockRequire("oxalis/model/bucket_data_handling/wkstore_adapter", wkstoreAdapter);

Expand Down
30 changes: 9 additions & 21 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,6 @@
"url": "git://github.com/scalableminds/webknossos.git"
},
"devDependencies": {
"@babel/cli": "^7.1.0",
"@babel/core": "^7.1.0",
"@babel/plugin-proposal-class-properties": "^7.1.0",
"@babel/plugin-proposal-object-rest-spread": "^7.0.0",
"@babel/plugin-syntax-dynamic-import": "^7.0.0",
"@babel/polyfill": "^7.0.0",
"@babel/preset-env": "^7.1.0",
"@babel/preset-react": "^7.0.0",
"@babel/preset-typescript": "^7.16.7",
"@babel/register": "^7.0.0",
"@shaderfrog/glsl-parser": "^0.2.2",
"@types/color-hash": "^1.0.2",
"@types/cwise": "^1.0.4",
Expand All @@ -41,10 +31,7 @@
"@typescript-eslint/parser": "^5.40.1",
"abort-controller": "^3.0.0",
"ava": "^3.13.0",
"babel-eslint": "^9.0.0",
"babel-loader": "^8.2.3",
"babel-plugin-import": "^1.9.1",
"babel-plugin-istanbul": "^5.1.0",
"browserslist-to-esbuild": "^1.2.0",
"commander": "^2.17.1",
"coveralls": "^3.0.2",
"css-loader": "^6.5.1",
Expand All @@ -68,7 +55,6 @@
"merge-img": "^2.1.2",
"mock-require": "^1.2.1",
"node-fetch": "^2.6.7",
"nyc": "^13.3.0",
"pg": "^7.4.1",
"pixelmatch": "^5.2.0",
"pngjs": "^3.3.3",
Expand Down Expand Up @@ -127,8 +113,8 @@
"refresh-schema": "./tools/postgres/refresh_schema.sh && rm -f target/scala-2.12/src_managed/schema/com/scalableminds/webknossos/schema/Tables.scala",
"enable-jobs": "sed -i -e 's/jobsEnabled = false/jobsEnabled = true/g' ./conf/application.conf; ./tools/postgres/set_jobs.sh true",
"disable-jobs": "sed -i -e 's/jobsEnabled = true/jobsEnabled = false/g' ./conf/application.conf; ./tools/postgres/set_jobs.sh false",
"coverage-local": "nyc report --reporter=html && echo Success! Open coverage/index.html",
"coverage": "nyc report --reporter=text-lcov | coveralls",
"coverage-local": "c8 report --reporter=html && echo Success! Open coverage/index.html",
"coverage": "c8 report --reporter=text-lcov | coveralls",
"postcheckout": "echo 'Deleting auto-generated typescript files...' && rm -f frontend/javascripts/test/snapshots/type-check/*.ts",
"apply-evolutions": "tools/postgres/apply_evolutions.sh",
"postinstall": "cd tools/proxy && yarn install",
Expand Down Expand Up @@ -166,6 +152,7 @@
"ball-morphology": "^0.1.0",
"base64-js": "^1.2.1",
"beautiful-react-hooks": "^3.11.1",
"c8": "^7.12.0",
"chalk": "^5.0.1",
"classnames": "^2.2.5",
"color-hash": "^2.0.1",
Expand All @@ -178,6 +165,8 @@
"deep-freeze": "0.0.1",
"dice-coefficient": "^2.1.0",
"distance-transform": "^1.0.2",
"esbuild": "^0.16.10",
"esbuild-loader": "^2.20.0",
"file-saver": "^2.0.1",
"flexlayout-react": "^0.5.5",
"hammerjs": "^2.0.8",
Expand Down Expand Up @@ -254,12 +243,11 @@
"snapshotDir": "frontend/javascripts/test/snapshots",
"concurrency": 8
},
"nyc": {
"sourceMap": false,
"instrument": false,
"c8": {
"exclude": [
"public-test/test-bundle/test/**/*.*",
"frontend/javascripts/test/**/*.*"
]
],
"reporter": "lcov"
}
}
40 changes: 30 additions & 10 deletions tools/test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,30 @@ function prepare {
# Webpack with the proto loader isn't used when running the tests, so the proto files need to be prepared manually
pbjs -t json "webknossos-datastore/proto/SkeletonTracing.proto" > "$testBundlePath/SkeletonTracing.proto.json"
pbjs -t json "webknossos-datastore/proto/VolumeTracing.proto" > "$testBundlePath/VolumeTracing.proto.json"
# --copy-files will copy files that are present in the source dir but are not transpiled (e.g.: json files)
# "$@" inside this function refers to all arguments of the prepare function, not all arguments of this bash script
BABEL_ENV=test babel --extensions .ts,.tsx "$jsPath" --out-dir "$testBundlePath" --copy-files "$@"

# Beginning from target==node13, dynamic imports are not converted anymore by esbuild. Tests which use code
# that relies on dynamic imports fails then because the module cannot be found for some reason.
node_modules/.bin/esbuild \
--platform=node \
--format=cjs \
--define:process.env.IS_TESTING=\"true\" \
--target=node12 \
--outdir="$testBundlePath" $($FIND frontend/javascripts \( -name "*.ts" -o -name "*.tsx" \))

# Copy files which were not compiled by esbuild (e.g., snapshots).
find frontend/javascripts -type f -not \( -name "*.ts" -o -name "*.tsx" -o -name "*.png" \) -exec sh -c '
testBundlePath="public-test/test-bundle"
file="$1" # E.g., frontend/javascripts/test/snapshots/public-test/test-bundle/test/libs/nml.spec.js.snap
from="$file"
to="$file"
to=${to#*/} # Remove everything until (and including) the first / to trim the first folder. E.g., javascripts/test/snapshots/public-test/test-bundle/test/libs/nml.spec.js.snap
to=${to#*/} # Also remove the second folder. E.g., test/snapshots/public-test/test-bundle/test/libs/nml.spec.js.snap
to="$testBundlePath/$to" # Add new bundle path as parent. E.g. public-test/test-bundle/test/snapshots/public-test/test-bundle/test/libs/nml.spec.js.snap
to_dir=${to%/*} # Remove the file name from the path in $to. E.g., public-test/test-bundle/test/snapshots/public-test/test-bundle/test/libs
# Only copy when src and dst differ
cmp --silent $from $to && echo skip $from to $to
cmp --silent $from $to || mkdir -p $to_dir && cp $from $to
' find-sh {} \;
}

function ensureUpToDateTests {
Expand All @@ -39,32 +60,31 @@ function ensureUpToDateTests {
fi
}

# For faster, local testing, you may want to remove the `nyc` part of the following statement.
# Also, removing `istanbul` from .babelrc, allows to debug uninstrumented source code.
# For faster, local testing, you may want to remove the `c8` part of the following statement.
if [ $cmd == "test" ]
then
ensureUpToDateTests
export NODE_PATH="$testBundlePath" && BABEL_ENV=test nyc --silent --no-clean --exclude binaryData ava $(find "$testBundlePath" -name "*.spec.js") "$@"
export NODE_PATH="$testBundlePath" && c8 --silent --no-clean --exclude binaryData ava $(find "$testBundlePath" -name "*.spec.js") "$@"
elif [ $cmd == "test-debug" ]
then
export NODE_PATH="$testBundlePath" && BABEL_ENV=test ava debug $(find "$testBundlePath" -name "*.spec.js") "$@"
export NODE_PATH="$testBundlePath" && ava debug $(find "$testBundlePath" -name "*.spec.js") "$@"
elif [ $cmd == "test-changed" ]
then
ensureUpToDateTests
# Find modified *.spec.* files, trim their extension (since ts != js) and look them up in the compiled bundle
changed_files=$(git ls-files --modified | grep \.spec\. | xargs -i basename {} | sed -r 's|^(.*?)\.\w+$|\1|' | xargs -i find public-test/test-bundle -name "{}*")
echo Only running $changed_files
export NODE_PATH="$testBundlePath" && BABEL_ENV=test nyc --silent --no-clean --exclude binaryData ava \
export NODE_PATH="$testBundlePath" && c8 --silent --no-clean --exclude binaryData ava \
$changed_files \
"$@"
elif [ $cmd == "test-e2e" ]
then
ensureUpToDateTests
export NODE_PATH="$testBundlePath" && BABEL_ENV=test nyc --silent --no-clean ava $(find "$testBundlePath" -name "*.e2e.js") --serial -C 1 "$@"
export NODE_PATH="$testBundlePath" && c8 --silent --no-clean ava $(find "$testBundlePath" -name "*.e2e.js") --serial -C 1 "$@"
elif [ $cmd == "test-screenshot" ]
then
ensureUpToDateTests
export NODE_PATH="$testBundlePath" && BABEL_ENV=test ava $(find "$testBundlePath" -name "*.screenshot.js") "$@"
export NODE_PATH="$testBundlePath" && ava $(find "$testBundlePath" -name "*.screenshot.js") "$@"
elif [ $cmd == "prepare" ]
then
prepare "$@"
Expand Down
13 changes: 12 additions & 1 deletion webpack.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ module.exports = function (env = {}) {
const path = require("path");
const MiniCssExtractPlugin = require("mini-css-extract-plugin");
const TerserPlugin = require("terser-webpack-plugin");
const browserslistToEsbuild = require("browserslist-to-esbuild");

const srcPath = path.resolve(__dirname, "frontend/javascripts/");
const nodePath = "node_modules";
Expand Down Expand Up @@ -77,7 +78,17 @@ module.exports = function (env = {}) {
{
test: /\.tsx?$/,
exclude: /(node_modules|bower_components)/,
use: "babel-loader",
loader: "esbuild-loader",
options: {
loader: "tsx", // also supports 'ts'
target: browserslistToEsbuild([
"last 3 Chrome versions",
"last 3 Firefox versions",
"last 2 Edge versions",
"last 1 Safari versions",
"last 1 iOS versions",
]),
},
},
{
test: /\.less$/,
Expand Down
Loading

0 comments on commit 5995054

Please sign in to comment.