[dagit] Upgrade to Jest 29, allow more time for coverage collection (#…

…12534) ### Summary & Motivation The goal of this PR is to resolve sporadic test failures due to Jest running ouf out-of-memory or running too slowly. - Upgraded to Jest 29 and configured it to restart it's worker if it ever uses more then 1GB of RAM - Increased the test timeout to 10s. The tests run considerably more slowly with `--coverage` enabled, especially tests that "cover" a large amount of code. My AssetPartitions tests render the whole page, which covers dozens of files, and adding --coverage pushes that test over the 5s timeout limit randomly. We could also disable coverage, but it seems nice to have? (Have we been viewing it somewhere?) --------- Co-authored-by: bengotow <bgotow@elementl.com>
dagster-io · Feb 24, 2023 · 102c0bc · 102c0bc · vercel · Feb 24, 2023
1 parent 7875f44
commit 102c0bc
Show file tree

Hide file tree

Showing 12 changed files with 833 additions and 1,528 deletions.
diff --git a/.buildkite/dagster-buildkite/dagster_buildkite/steps/dagit_ui.py b/.buildkite/dagster-buildkite/dagster_buildkite/steps/dagit_ui.py
@@ -36,8 +36,6 @@ def build_dagit_ui_steps() -> List[CommandStep]:
             "curl -sL https://deb.nodesource.com/setup_16.x | bash -",
             "apt-get -yqq --no-install-recommends install nodejs",
             "tox -vv -e py39",
-            "mv packages/core/coverage/lcov.info lcov.dagit.$BUILDKITE_BUILD_ID.info",
-            "buildkite-agent artifact upload lcov.dagit.$BUILDKITE_BUILD_ID.info",
         )
         .on_test_image(AvailablePythonVersion.get_default())
         .with_skip(skip_if_no_dagit_changes())

diff --git a/js_modules/dagit/packages/core/jest.config.js b/js_modules/dagit/packages/core/jest.config.js
@@ -134,10 +134,15 @@ module.exports = {
   // snapshotSerializers: [],
 
   // The test environment that will be used for testing
-  // testEnvironment: "jest-environment-jsdom",
+  testEnvironment: 'jsdom',
 
   // Options that will be passed to the testEnvironment
-  // testEnvironmentOptions: {},
+  // https://jestjs.io/docs/configuration#testenvironmentoptions-object
+  // Even though we're loading in a "JSDOM" compatible environment, we want to load the /node/ versions of
+  // modules that are required, not the browser-ready versions.
+  testEnvironmentOptions: {
+    customExportConditions: ['node', 'node-addons'],
+  },
 
   // Adds a location field to test results
   // testLocationInResults: false,

diff --git a/js_modules/dagit/packages/core/jest/fileTransformer.js b/js_modules/dagit/packages/core/jest/fileTransformer.js
@@ -2,7 +2,7 @@
 const path = require('path');
 
 module.exports = {
-  process(src, filename, _config, _options) {
-    return 'module.exports = ' + JSON.stringify(path.basename(filename)) + ';';
+  process(_src, filename, _config, _options) {
+    return {code: 'module.exports = ' + JSON.stringify(path.basename(filename)) + ';'};
   },
 };
diff --git a/js_modules/dagit/packages/core/package.json b/js_modules/dagit/packages/core/package.json
@@ -127,9 +127,9 @@
     "graphql-tag": "^2.10.1",
     "graphql.macro": "^1.4.2",
     "identity-obj-proxy": "^3.0.0",
-    "jest": "^26.6.3",
+    "jest": "^29.4",
     "jest-canvas-mock": "^2.4.0",
-    "jest-environment-jsdom-sixteen": "^1.0.3",
+    "jest-environment-jsdom": "^29.4",
     "prettier": "2.2.1",
     "react": "^17.0.2",
     "react-dom": "^17.0.2",

diff --git a/js_modules/dagit/packages/core/src/assets/usePartitionHealthData.tsx b/js_modules/dagit/packages/core/src/assets/usePartitionHealthData.tsx
@@ -88,11 +88,11 @@ export function buildPartitionHealthData(data: PartitionHealthQuery, loadKey: As
 
   const stateForKeyWithRangeOrdering = (dimensionKeys: string[]): PartitionState => {
     if (dimensionKeys.length !== dimensions.length) {
-      console.warn('[stateForKey] called with incorrect number of dimensions');
+      warnUnlessTest('[stateForKey] called with incorrect number of dimensions');
       return PartitionState.MISSING;
     }
     if (dimensionKeys.length === 0) {
-      console.warn('[stateForKey] called with zero dimension keys');
+      warnUnlessTest('[stateForKey] called with zero dimension keys');
       return PartitionState.MISSING;
     }
 
@@ -119,7 +119,7 @@ export function buildPartitionHealthData(data: PartitionHealthQuery, loadKey: As
       return [];
     }
     if (dimensionIdx >= dimensions.length) {
-      console.warn('[rangesForSingleDimension] called with invalid dimension index');
+      warnUnlessTest('[rangesForSingleDimension] called with invalid dimension index');
       return [];
     }
 
@@ -289,7 +289,7 @@ export function keyCountByStateInSelection(
   selections: PartitionDimensionSelection[],
 ) {
   if (selections.length === 0) {
-    console.warn('[keyCountByStateInSelection] A selection must be provided for dimension 0.');
+    warnUnlessTest('[keyCountByStateInSelection] A selection must be provided for dimension 0.');
     return {
       [PartitionState.MISSING]: 0,
       [PartitionState.SUCCESS]: 0,
@@ -354,7 +354,7 @@ function addKeyIndexesToMaterializedRanges(
       });
     } else if (range.__typename === 'MaterializedPartitionRange2D') {
       if (dimensions.length !== 2) {
-        console.warn('[addKeyIndexesToMaterializedRanges] Found 2D health data for 1D asset');
+        warnUnlessTest('[addKeyIndexesToMaterializedRanges] Found 2D health data for 1D asset');
         return result;
       }
       const [dim0, dim1] = dimensions;
@@ -525,3 +525,8 @@ export const PARTITION_HEALTH_QUERY = gql`
     }
   }
 `;
+function warnUnlessTest(msg: string) {
+  if (process.env.NODE_ENV !== 'test') {
+    console.warn(msg);
+  }
+}
diff --git a/js_modules/dagit/packages/core/src/gantt/useViewport.tsx b/js_modules/dagit/packages/core/src/gantt/useViewport.tsx
@@ -66,7 +66,9 @@ export const useViewport = (
         });
         resizeObserver.observe(element);
       } else {
-        console.warn(`No ResizeObserver support, or useViewport is attached to a non-DOM node?`);
+        if (process.env.NODE_ENV !== 'test') {
+          console.warn(`No ResizeObserver support, or useViewport is attached to a non-DOM node?`);
+        }
         onApplySize({width: element.clientWidth, height: element.clientHeight});
       }
     }

diff --git a/js_modules/dagit/packages/eslint-config/package.json b/js_modules/dagit/packages/eslint-config/package.json
@@ -28,7 +28,7 @@
   "devDependencies": {
     "@types/jest": "^27.5.1",
     "eslint": "^8.6.0",
-    "jest": "^28.1.0",
+    "jest": "^29.4",
     "prettier": "2.2.1",
     "ts-jest": "^28.0.3",
     "typescript": "4.8.4"

diff --git a/js_modules/dagit/packages/ui/jest.config.js b/js_modules/dagit/packages/ui/jest.config.js
@@ -133,10 +133,15 @@ module.exports = {
   // snapshotSerializers: [],
 
   // The test environment that will be used for testing
-  testEnvironment: 'jest-environment-jsdom',
+  testEnvironment: 'jsdom',
 
   // Options that will be passed to the testEnvironment
-  // testEnvironmentOptions: {},
+  // https://jestjs.io/docs/configuration#testenvironmentoptions-object
+  // Even though we're loading in a "JSDOM" compatible environment, we want to load the /node/ versions of
+  // modules that are required, not the browser-ready versions.
+  testEnvironmentOptions: {
+    customExportConditions: ['node', 'node-addons'],
+  },
 
   // Adds a location field to test results
   // testLocationInResults: false,

diff --git a/js_modules/dagit/packages/ui/jest/fileTransformer.js b/js_modules/dagit/packages/ui/jest/fileTransformer.js
@@ -2,7 +2,7 @@
 const path = require('path');
 
 module.exports = {
-  process(src, filename, config, options) {
-    return 'module.exports = ' + JSON.stringify(path.basename(filename)) + ';';
+  process(_src, filename, _config, _options) {
+    return {code: 'module.exports = ' + JSON.stringify(path.basename(filename)) + ';'};
   },
 };
diff --git a/js_modules/dagit/packages/ui/package.json b/js_modules/dagit/packages/ui/package.json
@@ -82,7 +82,8 @@
     "core-js": "^3.21.1",
     "eslint": "^8.6.0",
     "eslint-plugin-storybook": "^0.5.5",
-    "jest": "^27.4.7",
+    "jest": "^29.4",
+    "jest-environment-jsdom": "^29.4",
     "nearest-color": "^0.4.4",
     "prettier": "2.2.1",
     "regenerator-runtime": "^0.13.9",

diff --git a/js_modules/dagit/tox.ini b/js_modules/dagit/tox.ini
@@ -25,12 +25,12 @@ commands =
   yarn workspace @dagster-io/dagit-app lint
   yarn workspace @dagster-io/dagit-app ts
   yarn workspace @dagster-io/dagit-core ts
-  yarn workspace @dagster-io/dagit-core jest --clearCache
-  yarn workspace @dagster-io/dagit-core jest --collectCoverage --watchAll=false
+  yarn workspace @dagster-io/dagit-core jest --clearCache 
+  yarn workspace @dagster-io/dagit-core jest --testTimeout=10000 --ci --logHeapUsage --workerIdleMemoryLimit=1GB
   yarn workspace @dagster-io/dagit-core check-prettier
   yarn workspace @dagster-io/dagit-core check-lint
   yarn workspace @dagster-io/ui ts
   yarn workspace @dagster-io/ui lint
   yarn workspace @dagster-io/ui jest --clearCache
-  yarn workspace @dagster-io/ui jest
+  yarn workspace @dagster-io/ui jest --testTimeout=10000 --ci --logHeapUsage --workerIdleMemoryLimit=1GB
   git diff --exit-code