OLMIS-8298: Run the React component specs as part of the karma suite - #44
Open
denys1204 wants to merge 1 commit into
Open
OLMIS-8298: Run the React component specs as part of the karma suite#44denys1204 wants to merge 1 commit into
denys1204 wants to merge 1 commit into
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
karma is fed plain script tags (
.tmp/javascript/src/**/*.jsplus the copied*.spec.jsfiles), so the.jsxsources never reach it. They are ES modules containing JSX and only get transpiled on the way intothe webpack app bundle, which karma explicitly excludes. Two consequences: a spec written next to a React
component never runs, and every
.jsxfile reports no coverage at all, which drags the SonarCloud figuredown in every repo that has React code.
Krzysztof proved the fix in openlmis-ui-components (commit
928dfa2) by keeping the grunt tasks in thatrepository, 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:reactcollects*.spec.jsxfrom the src directory of every application directory (the same settest:copyalready walks for Angular specs), bundles them with webpack including React and everything elsethey import, instruments the sources they pull in, and hands the bundle to karma as one more file.
testisnow
['test:copy', 'test:react', 'karma:unit']. An application with no React specs logsNo 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.
retainLineskeeps statements ontheir original lines, so the lcov line numbers point back into the
.jsx.What differs from the ui-components proof of concept, and why
matches what
test:copyalready does for*.spec.js.require.resolve, because the task now lives in/dev-uirather thanin the application, and
/dev-ui/node_moduleswas added as a fallback for the bundle's own resolution.karma.options.fileswhen the task runs rather than when it is configured, sorepos 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 anAMD loader first, webpack answers that check truthfully, and that branch publishes them on the window.
openlmis-quantity-unit-input.jsximports lodash, so loading the spec bundle ranroot._ = _and replacedthe global underscore the Angular sources rely on. The application bundle never notices, because
src/index.jslocks_down (OE-138) and karma excludesindex.js. openlmis-ui-components' own suitestayed green because nothing in it calls a helper lodash 4 dropped, but a reference-ui build lost 151 specs
to
_.any is not a functionand_.findWhere is not a functionacross stockmanagement and referencedata.With AMD off those libraries stay on their CommonJS branch and touch nothing global.
Verified locally
Executed 1003 of 1003 SUCCESS, the component at 100% linecoverage (LF 49 / LH 49) and 6
.jsxfiles in lcov. The same numbers its CI already produces with thetasks living in that repo.
Executed 4618 of 4618 SUCCESS, the ui-components React specbundled out of
/openlmis-ui-components/srcand its 28 cases green.*.spec.jsxremoved: the skip message, 975 tests, no karma warning and no bundle built.node_modules/dev-uito/dev-ui): every module the change requiresresolves from
/dev-ui/node_modules, istanbul included, and the run is 1003/1003./dev-ui/istanbul-loader.jsand/dev-ui/tasks/react-tests.jscorrectly, verified with aCOPY-only build and checked inside a running container so the
VOLUMEdeclaration is accounted for.Two things worth knowing before merging
Merge order: this has to be published as
openlmis/dev-ui:9.0.9-SNAPSHOTbefore the openlmis-ui-componentschange that deletes its local copy of the tasks, otherwise that repo drops back to 975 tests and loses the
.jsxcoverage again. Its CI run says which image it got: 1003 tests is this one, 975 is the old one.A full
docker buildof this image currently fails on a cold cache, for a reason that predates this change.Line 15 fetches the retired NodeSource
node_14.xapt repo, whose signing key is no longer served, so aptrefuses the unsigned repository. The pre-change Dockerfile fails identically at the same step. A warm agent
should reuse that layer and reach our
COPYat line 41, but if the publish fails this is why, and it needsits own ticket.
The task requires
istanbulthrough karma-coverage's own dependency rather than declaring it here. That isdeliberate: the point is to use the same instrumenter karma-coverage summarises, and a separate declaration
could drift away from it. The existing tasks require
globthe same way.