Added Jest test coverage and refactored demo test to use 'require()'.#2446
Added Jest test coverage and refactored demo test to use 'require()'.#2446AlexSkrypnyk merged 1 commit intomainfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (30)
📒 Files selected for processing (7)
WalkthroughEnabled Jest coverage collection and reports, moved tests into Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer / CLI
participant Jest as Jest Runner
participant Loader as Node Module Loader
participant Module as ys_demo.js
participant Reporter as Coverage Reporter
participant FS as Filesystem
Dev->>Jest: run `npm test` (jest --coverage)
Jest->>Jest: jest.resetModules()
Jest->>Loader: require('../ys_demo.js')
Loader->>Module: load & execute module IIFE
Module-->>Jest: behavior registered / exports available
Jest->>Reporter: collect coverage data during tests
Reporter->>FS: write reports to .logs/coverage/jest/ (text, lcov, html, cobertura.xml)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@jest.config.js`:
- Line 34: The Jest config writes coverage to coverageDirectory
'.logs/coverage/jest' which CI does not read; update the jest.config.js
coverageDirectory to the CI artifacts path (e.g. '/tmp/artifacts/coverage/jest')
and ensure Jest emits the required formats by adding/updating the
coverageReporters entry to include 'cobertura' and 'text' (so the CI can consume
cobertura.xml and coverage.txt). Modify the coverageDirectory and
coverageReporters entries in jest.config.js accordingly.
- Line 33: The coverageReporters array formatting in jest.config.js violates
Prettier rules; reformat the coverageReporters value so it adheres to project
Prettier settings (e.g., split elements consistently and avoid nested array
line-wrapping that breaks rules) by adjusting the coverageReporters array
declaration (the coverageReporters key and its value that includes
['text','lcov','html', ['cobertura', { file: 'cobertura.xml' }]]) to a
Prettier-compliant layout.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 26ec5fc0-422a-480b-9ac2-4d49c580714f
📒 Files selected for processing (2)
jest.config.jspackage.json
| ], | ||
| collectCoverageFrom: ['web/modules/custom/**/js/**/*.js', '!**/*.test.js'], | ||
| coverageReporters: ['text', 'lcov', 'html', ['cobertura', { file: 'cobertura.xml' }]], | ||
| coverageDirectory: '.logs/coverage/jest', |
There was a problem hiding this comment.
Coverage output path is not wired into current CI coverage extraction.
Line 34 writes reports to .logs/coverage/jest, but CI currently extracts and enforces coverage from /tmp/artifacts/coverage/phpunit/cobertura.xml and /tmp/artifacts/coverage/phpunit/coverage.txt (.circleci/config.yml Lines 400-437). As-is, Jest coverage won’t be included in the coverage summary/threshold flow.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@jest.config.js` at line 34, The Jest config writes coverage to
coverageDirectory '.logs/coverage/jest' which CI does not read; update the
jest.config.js coverageDirectory to the CI artifacts path (e.g.
'/tmp/artifacts/coverage/jest') and ensure Jest emits the required formats by
adding/updating the coverageReporters entry to include 'cobertura' and 'text'
(so the CI can consume cobertura.xml and coverage.txt). Modify the
coverageDirectory and coverageReporters entries in jest.config.js accordingly.
There was a problem hiding this comment.
The Jest coverage output at .logs/coverage/jest follows the project convention where all coverage reports live under .logs/coverage/<tool>. The CI PHPUnit coverage paths are specific to PHPUnit — Jest coverage collection is new and its CI integration will be handled separately when coverage thresholds are configured for JavaScript.
There was a problem hiding this comment.
@AlexSkrypnyk, understood — the .logs/coverage/<tool> layout is the project-wide convention, and the PHPUnit CI paths are specific to that tool's existing integration. I'll keep this in mind for future reviews so I don't flag Jest coverage paths as misaligned with the PHPUnit CI setup.
✏️ Learnings added
Learnt from: AlexSkrypnyk
Repo: drevops/vortex PR: 2446
File: jest.config.js:34-34
Timestamp: 2026-04-02T07:57:34.712Z
Learning: In the drevops/vortex repository, all coverage reports follow the convention `.logs/coverage/<tool>` (e.g. `.logs/coverage/jest` for Jest, `.logs/coverage/phpunit` for PHPUnit). The CI PHPUnit coverage extraction paths (e.g. `/tmp/artifacts/coverage/phpunit/cobertura.xml`) are specific to PHPUnit. Jest CI coverage integration is separate and will be configured independently. Do not flag `.logs/coverage/jest` as misaligned with PHPUnit CI artifact paths.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
2 similar comments
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.vortex/tests/phpunit/Traits/Subtests/SubtestAhoyTrait.php:
- Around line 545-551: The tests currently use
assertFileContainsString('.logs/coverage/jest/cobertura.xml', 'line-rate="1"')
and similar for branch-rate, which forces 100% coverage; change these to instead
verify the attributes exist with any numeric value by reading the file (e.g.,
via file_get_contents) and using a regex-based assertion (e.g.,
assertMatchesRegularExpression or assertFileMatchesRegularExpression) against
patterns like /line-rate="\d+(\.\d+)?"/ and /branch-rate="\d+(\.\d+)?"/; update
the assertions in the test method that calls logSubstep('Assert Jest coverage
files are present') and syncToHost('.logs') to use these regex checks instead of
exact '="1"' matches so the test asserts coverage metadata presence without
requiring perfect coverage.
In `@web/modules/custom/ys_demo/js/tests/ys_demo.test.js`:
- Line 14: The test require statement require('../ys_demo.js') triggers an
ESLint import/extensions warning; either remove the extension in the test to use
Node's default resolution (change the require call in ys_demo.test.js to
require('../ys_demo')) or add an ESLint override to disable import/extensions
for tests (update .eslintrc.json to include an override for the test glob that
sets "import/extensions": "off"); locate the require in ys_demo.test.js and
apply one of these two fixes consistently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6792b438-5feb-4d08-b549-14d44766ec61
📒 Files selected for processing (5)
.eslintrc.json.vortex/docs/content/development/jest.mdx.vortex/tests/phpunit/Traits/Subtests/SubtestAhoyTrait.phpweb/modules/custom/ys_demo/js/tests/ys_demo.test.jsweb/modules/custom/ys_demo/js/ys_demo.js
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
2 similar comments
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
ac45c3b to
1261396
Compare
|
Code coverage (threshold: 90%) Per-class coverage |
This comment has been minimized.
This comment has been minimized.
2 similar comments
This comment has been minimized.
This comment has been minimized.
|
Code coverage (threshold: 90%) Per-class coverage |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2446 +/- ##
==========================================
- Coverage 79.78% 79.42% -0.36%
==========================================
Files 127 121 -6
Lines 6825 6702 -123
Branches 44 3 -41
==========================================
- Hits 5445 5323 -122
+ Misses 1380 1379 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary
Added Jest test coverage collection and refactored the demo test to use
require()instead ofeval()for loading Drupal behaviors. Moved test files to atests/subdirectory, added alog()wrapper method to suppress console output during tests, updated ESLint config, functional test assertions, and documentation.Changes
Jest coverage collection
jest.config.js— AddedcollectCoverageFrom,coverageReporters(text, lcov, HTML, Cobertura), andcoverageDirectorytargeting.logs/coverage/jestpackage.json— Added--coverageflag to thetestscriptDemo test refactoring
web/modules/custom/ys_demo/js/tests/ys_demo.test.js— Replacedeval(fs.readFileSync(...))withjest.resetModules()+require()for loading behaviors; addedjest.spyOn(console, 'log')to suppress output during tests; moved fromjs/tojs/tests/subdirectoryweb/modules/custom/ys_demo/js/ys_demo.js— Extractedconsole.logcall into alog()method on the behavior objectESLint and config
.eslintrc.json— Replacedno-eval: offwithglobal-require: offin the*.test.jsoverrideFunctional test assertions
.vortex/tests/phpunit/Traits/Subtests/SubtestAhoyTrait.php— Updated test file path tojs/tests/; added assertions for coverage directory,cobertura.xmlfile,lcov-reportdirectory, and 100% line/branch rates in Cobertura XMLDocumentation
.vortex/docs/content/development/jest.mdx— Updated test file structure to showtests/subdirectory; updated template to userequire()pattern; added Coverage section documenting reporters and output locations; updated ESLint and boilerplate sectionsSummary by CodeRabbit
Tests
.logs/coverage/jest/directory when running tests.Documentation
Chores