Merge pull request #475 from roy232828/improve-cypress-ci-speed

Improve cypress ci speed

Author: roy232828

.github/workflows/test.yml

@@ -1,9 +1,10 @@
 name: Test CI
 
-on: [push, pull_request]
+on: [ push, pull_request ]
 
 jobs:
   build-artifact:
+    name: Build And Test
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v4
@@ -29,6 +30,14 @@ jobs:
           restore-keys: |
             ${{ runner.os }}-modules-
 
+      - name: Restore Turbo cache
+        uses: actions/cache/restore@v4
+        id: restore-turbo-cache
+        with:
+          path: .turbo/cache
+          key: ${{ runner.os }}-turbo-${{ hashFiles('**/yarn.lock') }}
+          restore-keys: |
+            ${{ runner.os }}-turbo-
       - name: Restore Cypress cache
         uses: actions/cache/restore@v4
         id: restore-cypress-cache
@@ -47,10 +56,17 @@ jobs:
       - name: cypress install
         if: |
           steps.restore-cypress-cache.outputs.cache-hit != 'true'
-        run: yarn cypress install
-
-      - name: yarn build
-        run: yarn build
+        run: |
+          # run yarn cypress verify, if it fails, run yarn cypress install \
+          if ! yarn cypress verify; then \
+            yarn cypress install \
+          fi
+
+      - name: Build and Test
+        run: |
+          yarn turbo run build && \
+          # skipping testing ds-ext for now \
+          yarn turbo run test --filter=!ds-ext
 
       - name: Upload build dist artifacts
         uses: actions/upload-artifact@v4
@@ -78,7 +94,15 @@ jobs:
             !**/node_modules/.cache
             !**/node_modules/**/node_modules
           key: ${{ steps.restore-node-modules.outputs.cache-primary-key }}
-      
+
+      # Save Turbo cache (always execute)
+      - name: Save Turbo cache
+        if: success()
+        uses: actions/cache/save@v4
+        with:
+          path: .turbo/cache
+          key: ${{ steps.restore-turbo-cache.outputs.cache-primary-key }}
+
       # Save Cypress cache (always execute)
       - name: Save Cypress cache
         if: success()
@@ -90,95 +114,3 @@ jobs:
       - name: Cleanup
         if: always()
         run: yarn cache clean
-
-  unit-test:
-    needs: build-artifact
-    runs-on: ubuntu-latest
-    steps:
-      - uses: actions/checkout@v4
-
-      - name: Use Node.js
-        uses: actions/setup-node@v4
-        with:
-          node-version: '21.x'
-          registry-url: 'https://registry.npmjs.org'
-
-      - name: Restore node_modules
-        uses: actions/cache/restore@v4
-        id: restore-node-modules
-        with:
-          path: |
-            node_modules
-            packages/**/node_modules
-            !node_modules/@data-story
-            !packages/*/node_modules/@data-story
-            !**/node_modules/.cache
-            !**/node_modules/**/node_modules
-
-          key: ${{ runner.os }}-modules-${{ hashFiles('**/yarn.lock') }}
-          restore-keys: |
-            ${{ runner.os }}-modules-
-
-      - name: Restore Cypress cache
-        uses: actions/cache/restore@v4
-        id: restore-cypress-cache
-        with:
-          path: /home/runner/.cache/Cypress
-          key: ${{ runner.os }}-cypress-${{ hashFiles('**/yarn.lock') }}
-          restore-keys: |
-            ${{ runner.os }}-cypress-
-
-      - name: Download dist artifacts
-        uses: actions/download-artifact@v4
-        with:
-          name: dist-artifacts
-          path: packages
-
-      - name: Run @data-story/core, @data-story/ui, @data-story/docs tests
-        run: yarn run ci:test-packages
-
-  e2e-test:
-    needs: build-artifact
-    runs-on: ubuntu-latest
-    steps:
-      - uses: actions/checkout@v4
-
-      - name: Use Node.js
-        uses: actions/setup-node@v4
-        with:
-          node-version: '21.x'
-          registry-url: 'https://registry.npmjs.org'
-
-      - name: Restore node_modules
-        uses: actions/cache/restore@v4
-        id: restore-node-modules
-        with:
-          path: |
-            node_modules
-            packages/**/node_modules
-            !node_modules/@data-story
-            !packages/*/node_modules/@data-story
-            !**/node_modules/.cache
-            !**/node_modules/**/node_modules
-
-          key: ${{ runner.os }}-modules-${{ hashFiles('**/yarn.lock') }}
-          restore-keys: |
-            ${{ runner.os }}-modules-
-
-      - name: Restore Cypress cache
-        uses: actions/cache/restore@v4
-        id: restore-cypress-cache
-        with:
-          path: /home/runner/.cache/Cypress
-          key: ${{ runner.os }}-cypress-${{ hashFiles('**/yarn.lock') }}
-          restore-keys: |
-            ${{ runner.os }}-cypress-
-
-      - name: Download build artifacts
-        uses: actions/download-artifact@v4
-        with:
-          name: dist-artifacts
-          path: packages
-
-      - name: Run e2e tests
-        run: yarn run ci:e2e

cypress.config.ts

@@ -1,23 +1,15 @@
 import { defineConfig } from 'cypress';
-import webpackConfig from './packages/ui/cypress-webpack.config';
+import bundlerConfig from './packages/ui/cypress-vite.config';
 
 export default defineConfig({
   component: {
     specPattern: '**/*.cy.{ts,tsx}',
     devServer: {
       framework: 'react',
-      bundler: 'webpack',
-      webpackConfig: webpackConfig,
+      bundler: 'vite',
+      viteConfig: bundlerConfig,
     },
     viewportWidth: 384,
     viewportHeight: 216,
   },
-
-  e2e: {
-    baseUrl: 'http://localhost:3009',
-    specPattern: 'cypress/e2e/**/*',
-    setupNodeEvents(on, config) {
-      // implement node event listeners here
-    },
-  },
 });

package.json

@@ -29,11 +29,9 @@
     "release-ui": "yarn workspace @data-story/ui run release",
     "release-nodejs": "yarn workspace @data-story/nodejs run release",
     "release-ds-ext": "yarn workspace ds-ext run release",
-    "ci:test": "yarn ci:test-packages && yarn ci:e2e",
+    "ci:test": "yarn ci:test-packages",
     "ci:test-packages": "turbo test --filter=@data-story/core --filter=@data-story/ui --filter=@data-story/docs",
-    "ci:e2e": "start-server-and-test 'turbo dev --parallel -- --port 3009' 3009 'cy:e2e' ",
     "core:test": "turbo test --filter=@data-story/core",
-    "cy:e2e": "cypress run --e2e",
     "cy:open": "cypress open",
     "constraints": "yarn constraints"
   },

packages/core/src/ExecutionMemoryFactory.ts

@@ -12,6 +12,7 @@ import { Hook } from './types/Hook'
 import { ItemValue } from './types/ItemValue'
 import { LinkId } from './types/Link'
 import { Node, NodeId } from './types/Node'
+import { withNodeExecutionErrorHandling } from './utils/withNodeExecutionErrorHandling'
 
 export class ExecutionMemoryFactory {
   constructor(
@@ -64,7 +65,7 @@ export class ExecutionMemoryFactory {
 
       // Initialize runner context
       const context = new NodeRunnerContext(node.id);
-      context.status = computer.run({
+      context.status = withNodeExecutionErrorHandling(computer.run.bind(computer), node)({
         input: inputDevice,
         output: outputDevice,
         params: this.makeParamsDevice(node, memory),

packages/core/src/ItemWithParams/ItemWithParams.ts

@@ -1,12 +1,12 @@
 import { ItemValue } from '../types/ItemValue';
-import { Param, EvaluatedParamValue } from '../Param';
+import { EvaluatedParamValue, Param } from '../Param';
 import { ParamEvaluator } from './ParamEvaluator';
 
 export const isItemWithParams = (item: ItemWithParams | unknown): item is ItemWithParams => {
   // This does not always catch all cases
-  if(item instanceof ItemWithParams) return true;
+  if (item instanceof ItemWithParams) return true;
 
-  if(
+  if (
     item !== null
     && typeof item === 'object'
     // @ts-ignore
@@ -16,10 +16,10 @@ export const isItemWithParams = (item: ItemWithParams | unknown): item is ItemWi
   ) return true;
 
   return false;
-}
+};
 
 export class ItemWithParams<ExpectedType extends ItemValue = ItemValue> {
-  type = 'ItemWithParams' as const
+  type = 'ItemWithParams' as const;
   value: ExpectedType;
   params: Record<string, EvaluatedParamValue>;
 
@@ -37,11 +37,11 @@ export class ItemWithParams<ExpectedType extends ItemValue = ItemValue> {
         try {
           const paramEvaluatorInstance = new ParamEvaluator();
           return paramEvaluatorInstance.evaluate(value, param, globalParams);
-        } catch (error) {
-          console.error('error', error);
-          return param.input;
+        } catch (e) {
+          console.error('Failed while evaluating param', param, e);
+          throw e;
         }
       },
     });
   }
-}
+}

packages/core/src/index.ts

@@ -59,4 +59,4 @@ export type { Param, ParamInput as ParamValue } from './Param'
 export type { Port, AbstractPort } from './types/Port'
 export type { ReportCallback } from './types/ReportCallback';
 export type { ServiceProvider } from './types/ServiceProvider'
-export { Table } from './computers'
+export { Table } from './computers'

packages/core/src/types/Errors.ts

@@ -0,0 +1,8 @@
+import { Node } from './Node';
+
+export class NodeExecutionError extends Error {
+  constructor(message: string, public node: Pick<Node, 'id' | 'label' | 'type'>) {
+    super(`Error in node ${node.label || node.id} (${node.type}): ${message}`);
+    this.name = 'NodeExecutionError';
+  }
+}

packages/core/src/utils/withNodeExecutionErrorHandling.test.ts

@@ -0,0 +1,90 @@
+import { describe, expect, it } from 'vitest';
+import { withNodeExecutionErrorHandling } from './withNodeExecutionErrorHandling';
+import { NodeExecutionError } from '../types/Errors';
+
+describe('withNodeExecutionErrorHandling', () => {
+  const mockNode = {
+    id: 'test-node-1',
+    label: 'Test Node',
+    type: 'TestComputer',
+  };
+
+  it('should yield values normally when no error occurs', async () => {
+    const mockAsyncGenerator = async function* () {
+      yield 'value1';
+      yield 'value2';
+      return 'final';
+    };
+
+    const wrappedGenerator = withNodeExecutionErrorHandling(mockAsyncGenerator, mockNode);
+    const generator = wrappedGenerator();
+
+    const result1 = await generator.next();
+    expect(result1.value).toBe('value1');
+    expect(result1.done).toBe(false);
+
+    const result2 = await generator.next();
+    expect(result2.value).toBe('value2');
+    expect(result2.done).toBe(false);
+
+    const result3 = await generator.next();
+    expect(result3.value).toBe('final');
+    expect(result3.done).toBe(true);
+  });
+
+  it('should wrap errors with NodeExecutionError', async () => {
+    const errorMessage = 'Something went wrong';
+    const mockAsyncGenerator = async function* () {
+      yield 'value1';
+      throw new Error(errorMessage);
+    };
+
+    const wrappedGenerator = withNodeExecutionErrorHandling(mockAsyncGenerator, mockNode);
+    const generator = wrappedGenerator();
+
+    const result1 = await generator.next();
+    expect(result1.value).toBe('value1');
+
+    await expect(generator.next()).rejects.toThrow(NodeExecutionError);
+
+    try {
+      await generator.next();
+    } catch (error) {
+      expect(error).toBeInstanceOf(NodeExecutionError);
+      expect((error as NodeExecutionError).node).toEqual(mockNode);
+      expect((error as NodeExecutionError).message).toContain(errorMessage);
+      expect((error as NodeExecutionError).message).toContain(mockNode.label);
+      expect((error as NodeExecutionError).message).toContain(mockNode.type);
+    }
+  });
+
+  it('should handle non-Error objects thrown', async () => {
+    const mockAsyncGenerator = async function* () {
+      throw 'string error';
+    };
+
+    const wrappedGenerator = withNodeExecutionErrorHandling(mockAsyncGenerator, mockNode);
+    const generator = wrappedGenerator();
+
+    await expect(generator.next()).rejects.toThrow(NodeExecutionError);
+
+    try {
+      await generator.next();
+    } catch (error) {
+      expect(error).toBeInstanceOf(NodeExecutionError);
+      expect((error as NodeExecutionError).message).toContain('string error');
+    }
+  });
+
+  it('should pass arguments correctly to the wrapped function', async () => {
+    const mockAsyncGenerator = async function* (arg1: string, arg2: number) {
+      yield `${arg1}-${arg2}`;
+    };
+
+    const wrappedGenerator = withNodeExecutionErrorHandling(mockAsyncGenerator, mockNode);
+    const generator = wrappedGenerator('test', 42);
+
+    const result = await generator.next();
+    expect(result.value).toBe('test-42');
+  });
+});

packages/core/src/utils/withNodeExecutionErrorHandling.ts

@@ -0,0 +1,24 @@
+import { NodeExecutionError } from '../types/Errors';
+import { Node } from '../types/Node';
+
+/**
+ * High-order helper function that wraps an async generator function
+ * and rethrows any error thrown by the run function wrapped with NodeExecutionError
+ *
+ * @param runFn - The async generator function to wrap
+ * @param node - The node context for error reporting
+ * @returns A wrapped async generator that handles errors
+ */
+export function withNodeExecutionErrorHandling<TArgs extends any[], TYield, TReturn, TNext>(
+  runFn: (...args: TArgs) => AsyncGenerator<TYield, TReturn, TNext>,
+  node: Pick<Node, 'id' | 'label' | 'type'>,
+) {
+  return async function* (...args: TArgs): AsyncGenerator<TYield, TReturn, TNext> {
+    try {
+      return yield* runFn(...args);
+    } catch (error) {
+      const errorMessage = error instanceof Error ? error.message : String(error);
+      throw new NodeExecutionError(errorMessage, node);
+    }
+  };
+}