Firefly-2113: Scope test suite and add React component testing

.github/workflows/test.yml

@@ -0,0 +1,82 @@
+name: Tests
+
+on:
+  pull_request:
+    branches: [dev]
+
+permissions:
+  contents: read
+
+concurrency:
+  group: tests-${{ github.event.pull_request.number }}
+  cancel-in-progress: true
+
+jobs:
+  test:
+    name: unit tests
+    runs-on: ubuntu-latest
+    timeout-minutes: 30
+
+    steps:
+      # compose.yml's build context is the checkout's parent, so the checkout
+      # has to live in a directory named `firefly`, same as build_publish.yml.
+      - uses: actions/checkout@v5
+        with:
+          path: firefly
+
+      - uses: docker/setup-buildx-action@v3
+
+      # buildx so the layers land in the Actions cache
+      - name: Build test image
+        uses: docker/build-push-action@v6
+        with:
+          context: .
+          file: firefly/docker/Dockerfile
+          target: base
+          tags: ipac/firefly-test-js:latest
+          load: true
+          cache-from: type=gha
+          cache-to: type=gha,mode=max
+
+      - name: JavaScript tests (jest)
+        id: js-tests
+        working-directory: firefly
+        run: docker compose run --rm test-js
+
+      - name: Upload reports
+        if: '!cancelled()'
+        uses: actions/upload-artifact@v7
+        with:
+          name: test-reports
+          path: firefly/build/dist/reports/firefly/
+          if-no-files-found: warn
+          retention-days: 14
+
+      - name: Summarize
+        if: '!cancelled()'
+        env:
+          COVERAGE: firefly/build/dist/reports/firefly/coverage-summary.json
+        run: |
+          {
+            echo "## Test results"
+            echo
+            echo "| suite | result |"
+            echo "| --- | --- |"
+            echo "| JavaScript (jest) | ${{ steps.js-tests.outcome }} |"
+
+            # Written by the json-summary reporter in __jest__/jest.base.config.js.
+            # Absent when the test step dies before jest writes its report.
+            if [ -f "$COVERAGE" ]; then
+              echo
+              echo "### Coverage"
+              echo
+              echo "| metric | covered | percent |"
+              echo "| --- | --- | --- |"
+              jq -r '.total as $t
+                     | ["statements","branches","functions","lines"][]
+                     | "| \(.) | \($t[.].covered)/\($t[.].total) | \($t[.].pct)% |"' "$COVERAGE"
+            else
+              echo
+              echo "_No coverage report was produced._"
+            fi
+          } >> "$GITHUB_STEP_SUMMARY"

__jest__/InitTest.js

@@ -1,4 +1,3 @@
-import * as jest from 'jest';
 import {bootstrapRedux} from '../src/firefly/js/core/ReduxFlux';
 import {getBootstrapRegistry} from '../src/firefly/js/core/BootstrapRegistry.js';
 import {initHandleExternalUpload} from '../src/firefly/js/ui/FileUploadProcessor';

__jest__/jest.base.config.js

@@ -3,11 +3,13 @@
 /* eslint-env node */
 
 module.exports = {
+    // jest 27 changed the default to 'node'; these suites assume a DOM.
+    'testEnvironment': 'jsdom',
     'verbose': true,
     'clearMocks': true,
     'collectCoverage': true,
     'coverageDirectory': '../../build/dist/reports/firefly',
-    'coverageReporters': ['lcov'],
+    'coverageReporters': ['lcov', 'json-summary'],
     'moduleFileExtensions': [
         'js',
         'jsx'

compose.yml

@@ -24,6 +24,27 @@ services:
       - /opt/work/firefly/build
       - /opt/work/firefly/jars/build
       - /opt/work/firefly/node_modules
+
+  # Jest-only run, used by the GitHub Actions PR check.
+  test-js:
+    build:
+      context: ../
+      dockerfile: firefly/docker/Dockerfile
+      target: base
+    image: ipac/firefly-test-js:latest
+    environment:
+      # This makes src/firefly/jest.config.js drop the test suites
+      # that read a firefly_test_data checkout.
+      FIREFLY_SKIP_TESTDATA: ${FIREFLY_SKIP_TESTDATA:-true}
+      # Prevent OOM due to jsdom workers.
+      JEST_MAX_WORKERS: ${JEST_MAX_WORKERS:-2}
+    command: bash -c "cd firefly && gradle :firefly:jsTest"
+    # The reports reach the host through the bind, so CI can upload them.
+    volumes:
+      - ../firefly:/opt/work/firefly
+      - /opt/work/firefly/.gradle
+      - /opt/work/firefly/node_modules
+
   dev:
     build:
       context: ../

package.json

@@ -67,8 +67,12 @@
     "@eslint/compat": "^1.1.0",
     "@stylistic/eslint-plugin-js": "~4.4",
     "@svgr/webpack": "^8.1.0",
+    "@testing-library/dom": "^10",
+    "@testing-library/jest-dom": "~6.9.1",
+    "@testing-library/react": "^16",
+    "@testing-library/user-event": "^14",
     "babel-core": "7.0.0-bridge.0",
-    "babel-jest": "^24.9",
+    "babel-jest": "^30",
     "babel-loader": "^9.1.3",
     "babel-plugin-lodash": "^3.3",
     "babel-plugin-react-compiler": "^1.0.0",
@@ -80,7 +84,8 @@
     "eslint-plugin-react-hooks": "^7.0.1",
     "globals": "~16.4",
     "ink-docstrap": "~1.3",
-    "jest": "^24.9",
+    "jest": "^30",
+    "jest-environment-jsdom": "^30",
     "jsdoc": "~4.0",
     "jsdoc-jsx": "~0.1",
     "publish-release": "~1.6",

src/firefly/jest.config.js

@@ -2,7 +2,20 @@
 
 const baseConfig = require('../../__jest__/jest.base.config');
 
+// These two suites read a firefly_test_data checkout at module load, not inside a test,
+// so a missing checkout fails the whole file instead of one test.
+// Set FIREFLY_SKIP_TESTDATA=true to skip them where the data is unavailable.
+const needsTestData = ['/__tests__/Projection-test\\.js$', '/__tests__/Wavelength-test\\.js$'];
+
 module.exports = {
     ...baseConfig,
-    //add overrides here (if any)
-};
+    // add overrides here (if any)
+
+    // jest replaces its default rather than merging, so '/node_modules/' has to be repeated.
+    testPathIgnorePatterns: [
+        '/node_modules/',
+        ...(process.env.FIREFLY_SKIP_TESTDATA === 'true' ? needsTestData : []),
+    ],
+
+    ...(process.env.JEST_MAX_WORKERS ? {maxWorkers: Number(process.env.JEST_MAX_WORKERS)} : {}),
+};

src/firefly/js/externalSource/__tests__/sprintf-test.js

@@ -106,9 +106,9 @@ describe('sprintf: ', () => {
         expect(sprintf('%.3f',  14480n))              .toBe('14480.000');
 
         // when precision may be lost, error is thrown.
-        expect(() => sprintf('%J',    1448045501351006139n)).toThrowError(TypeError);
-        expect(() => sprintf('%.20g', 1448045501351006139n)).toThrowError(TypeError);
-        expect(() => sprintf('%f',    1448045501351006139n)).toThrowError(TypeError);
+        expect(() => sprintf('%J',    1448045501351006139n)).toThrow(TypeError);
+        expect(() => sprintf('%.20g', 1448045501351006139n)).toThrow(TypeError);
+        expect(() => sprintf('%f',    1448045501351006139n)).toThrow(TypeError);
     });
 
 });

src/firefly/js/ui/__tests__/CloseButton-test.jsx

@@ -0,0 +1,28 @@
+/*
+ * License information at https://github.com/Caltech-IPAC/firefly/blob/master/License.txt
+ */
+/*eslint-env node, jest */
+
+import React from 'react';
+import '@testing-library/jest-dom';
+import {render, screen} from '@testing-library/react';
+import userEvent from '@testing-library/user-event';
+
+import {CloseButton} from '../CloseButton.jsx';
+
+describe('CloseButton', () => {
+
+    it('renders a button labeled Close', () => {
+        render(<CloseButton/>);
+        expect(screen.getByRole('button')).toHaveTextContent('Close');
+    });
+
+    it('invokes onClick when pressed', async () => {
+        const onClick = jest.fn();
+        render(<CloseButton onClick={onClick}/>);
+
+        await userEvent.click(screen.getByRole('button', {name: /close/i}));
+
+        expect(onClick).toHaveBeenCalledTimes(1);
+    });
+});