Skip to content

OLMIS-8298: Run the React component specs as part of the karma suite - #44

Open
denys1204 wants to merge 1 commit into
masterfrom
OLMIS-8298
Open

OLMIS-8298: Run the React component specs as part of the karma suite#44
denys1204 wants to merge 1 commit into
masterfrom
OLMIS-8298

Conversation

@denys1204

Copy link
Copy Markdown

Why

karma is fed plain script tags (.tmp/javascript/src/**/*.js plus the copied *.spec.js files), so the
.jsx sources never reach it. They are ES modules containing JSX and only get transpiled on the way into
the webpack app bundle, which karma explicitly excludes. Two consequences: a spec written next to a React
component never runs, and every .jsx file reports no coverage at all, which drags the SonarCloud figure
down in every repo that has React code.

Krzysztof proved the fix in openlmis-ui-components (commit 928dfa2) by keeping the grunt tasks in that
repository, and wrote on the ticket that the next step is to move them here so every UI module gets them
from the dependency instead of a copy. That is this change.

What it does

test:react collects *.spec.jsx from the src directory of every application directory (the same set
test:copy already walks for Angular specs), bundles them with webpack including React and everything else
they import, instruments the sources they pull in, and hands the bundle to karma as one more file. test is
now ['test:copy', 'test:react', 'karma:unit']. An application with no React specs logs
No React specs found, skipping. and is otherwise untouched.

The instrumentation deliberately goes through the same istanbul that karma-coverage 1.x uses.
babel-plugin-istanbul and friends emit the newer coverage format, which karma-coverage cannot summarise, so
that choice would break the whole report rather than just the React part. retainLines keeps statements on
their original lines, so the lcov line numbers point back into the .jsx.

What differs from the ui-components proof of concept, and why

  • Specs are collected per application directory, so a distro build runs the modules' React specs too. This
    matches what test:copy already does for *.spec.js.
  • Loaders and presets resolve through require.resolve, because the task now lives in /dev-ui rather than
    in the application, and /dev-ui/node_modules was added as a fallback for the bundle's own resolution.
  • The bundle is appended to karma.options.files when the task runs rather than when it is configured, so
    repos without React specs get no "pattern does not match any file" warning.
  • { parser: { amd: false } }. This one is a bug fix rather than a port detail. UMD libraries check for an
    AMD loader first, webpack answers that check truthfully, and that branch publishes them on the window.
    openlmis-quantity-unit-input.jsx imports lodash, so loading the spec bundle ran root._ = _ and replaced
    the global underscore the Angular sources rely on. The application bundle never notices, because
    src/index.js locks _ down (OE-138) and karma excludes index.js. openlmis-ui-components' own suite
    stayed green because nothing in it calls a helper lodash 4 dropped, but a reference-ui build lost 151 specs
    to _.any is not a function and _.findWhere is not a function across stockmanagement and referencedata.
    With AMD off those libraries stay on their CommonJS branch and touch nothing global.

Verified locally

  • openlmis-ui-components: 975 to 1003 tests, Executed 1003 of 1003 SUCCESS, the component at 100% line
    coverage (LF 49 / LH 49) and 6 .jsx files in lcov. The same numbers its CI already produces with the
    tasks living in that repo.
  • reference-ui with all 11 modules mounted: Executed 4618 of 4618 SUCCESS, the ui-components React spec
    bundled out of /openlmis-ui-components/src and its 28 cases green.
  • With the only *.spec.jsx removed: the skip message, 975 tests, no karma warning and no bundle built.
  • Loaded through the CI-shaped symlink (node_modules/dev-ui to /dev-ui): every module the change requires
    resolves from /dev-ui/node_modules, istanbul included, and the run is 1003/1003.
  • The image places /dev-ui/istanbul-loader.js and /dev-ui/tasks/react-tests.js correctly, verified with a
    COPY-only build and checked inside a running container so the VOLUME declaration is accounted for.

Two things worth knowing before merging

Merge order: this has to be published as openlmis/dev-ui:9.0.9-SNAPSHOT before the openlmis-ui-components
change that deletes its local copy of the tasks, otherwise that repo drops back to 975 tests and loses the
.jsx coverage again. Its CI run says which image it got: 1003 tests is this one, 975 is the old one.

A full docker build of this image currently fails on a cold cache, for a reason that predates this change.
Line 15 fetches the retired NodeSource node_14.x apt repo, whose signing key is no longer served, so apt
refuses the unsigned repository. The pre-change Dockerfile fails identically at the same step. A warm agent
should reuse that layer and reach our COPY at line 41, but if the publish fails this is why, and it needs
its own ticket.

The task requires istanbul through karma-coverage's own dependency rather than declaring it here. That is
deliberate: the point is to use the same instrumenter karma-coverage summarises, and a separate declaration
could drift away from it. The existing tasks require glob the same way.

The karma task feeds the browser plain script tags, so the .jsx sources
never reach it - they are ES modules containing JSX and only get
transpiled on the way into the webpack app bundle, which karma excludes.
Every .jsx file therefore reports no coverage at all, and a spec written
next to one never runs.

The new test:react task bundles the *.spec.jsx files of each application
directory with webpack, instrumenting the sources they pull in with the
same istanbul karma-coverage uses, and hands the bundle to karma as one
more file. Applications without React specs skip it.

This was proven in openlmis-ui-components, which carried the tasks in its
own repository; keeping them here lets every UI module report React
coverage without a copy of the build logic.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant