Skip to content

feat(middleware-code-coverage)!: Support UI5 CLI v5 with bundle handling and source maps - #619

Open
devtomtom wants to merge 5 commits into
mainfrom
poc-rework-middleware-code-coverage
Open

devtomtom wants to merge 5 commits into
mainfrom
poc-rework-middleware-code-coverage

Conversation

@devtomtom

Copy link
Copy Markdown
Member

JIRA: CPOUI5FOUNDATION-1350
JIRA: CPOUI5FOUNDATION-1203

@devtomtom
devtomtom force-pushed the poc-rework-middleware-code-coverage branch from f339a67 to 31b4e2f Compare October 19, 2026 09:33
…dle handling and source maps

BREAKING CHANGE: Rework the code-coverage middleware for UI5 CLI v5.
- Support input source maps and report coverage against original sources,
  resolving the input source map via sourceMappingURL
- Switch integration tests to UI5 CLI v5 only and expand coverage across
  projects and build configs

JIRA: CPOUI5FOUNDATION-1350
JIRA: CPOUI5FOUNDATION-1203
@devtomtom
devtomtom force-pushed the poc-rework-middleware-code-coverage branch from 31b4e2f to 596ac88 Compare October 19, 2026 09:46
@devtomtom devtomtom changed the title feat!: UI5 Middleware Code Coverage v3 feat(middleware-code-coverage)!: Support UI5 CLI v5 with bundle handling and source maps Oct 19, 2026
@devtomtom
devtomtom force-pushed the poc-rework-middleware-code-coverage branch from bd97294 to aff646c Compare October 19, 2026 11:20

@d3xter666 d3xter666 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The solution works as expected and I have tested the scenarios in the fixtures and whether they provide coverage and correct paths to the files.

As this is a breaking change it's worth mentioning why is it breaking.
For exmaple: specVersion 5 is not available in @ui5/cli version 4 as well as builtResources

@matz3 matz3 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a first review, but looks very good so far 👍🏻
I have tested it locally on macOS and want to test on Windows.

Comment thread packages/middleware-code-coverage/lib/middleware.js Outdated
Comment thread packages/middleware-code-coverage/lib/middleware.js Outdated
Comment thread packages/middleware-code-coverage/lib/middleware.js Outdated
// falling back to the `//@ui5-bundle` content marker — the tag is absent when the minify
// task is skipped, but the marker is always present.
const source = await matchedResource.getString();
if (isBundleResource(matchedResource, log) || isBundleSource(source)) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. If the tag is found, we do not have to read the source via matchedResource.getString(), right? This affects performance / memory consumption, which might become relevant for very large bundles.
  2. I do not see a direct connection of an excluded minify task and therefore the IsBundle tag not being present. I could reproduce it locally. I do not see a reason why the tag should be omitted in such a case, so this sounds like a bug to me.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Done.
  2. You're right, minify isn't the cause. Acc. to AI, the tag isn't present for pre-built build-manifest artifacts.

Comment thread packages/middleware-code-coverage/lib/util.js Outdated
…IsBundle tag is absent

And align sourceMappingURL handling with @ui5/builder
@devtomtom
devtomtom force-pushed the poc-rework-middleware-code-coverage branch from f1943e6 to c2fc717 Compare October 23, 2026 13:50
@matz3

matz3 commented Oct 23, 2026

Copy link
Copy Markdown
Member

I have tested it locally on macOS and want to test on Windows.

I also tested on Windows, incl. TypeScript and it works as expected. Rewriting the paths to strip the drive (e.g. c:\\) is needed and works as expected.

This branch has not been deployed

No deployments
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.

3 participants