morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[PR #2955] [CLOSED] test(browser): cover source-map original filename parser modes #5515

New issue

Closed

opened 2026-10-31 01:07:14 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:07:14 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/2955
Author: @dakjdakd
Created: 10/13/2026
Status: ❌ Closed

Base: main ← Head: fix/source-map-parser-modes

📝 Commits (3)

3fe4a20 fix(browser): preserve source-map original filename parser modes
22f89f5 Merge remote-tracking branch 'origin/main' into HEAD
cb6cde0 Merge remote-tracking branch 'origin/main' into fix/source-map-parser-modes

📊 Changes

1 file changed (+116 additions, -0 deletions)

View changed files

➕ tests/boundary/browser/webSourceMapParserModes.test.ts (+116 -0)

📄 Description

Summary

Add production-boundary regression coverage for filename-dependent parser modes in browser source maps. The source-map implementation from #2954 is already on main in f6905719c. The remaining change is one test file, tests/boundary/browser/webSourceMapParserModes.test.ts, which exercises that implementation through actual HTTP source-map fetching.

The tests cover ordinary and indexed maps, TypeScript and CommonJS originals, webpack:// source identities, and query/fragment suffixes that resemble other filename extensions. They verify both parser completeness and retained dependency edges while checking that the complete original source URL remains present in the returned evidence.

Related issue: #2954, already resolved by the implementation on main.

Problem and expected behavior

An original filename is part of the parser's input semantics. For example, main.ts can contain const value = <number>1, while a CommonJS main.cjs can contain a top-level return. Parsing either without the appropriate filename mode can mark a valid original as incomplete and discard its dependency edges. Conversely, enabling permissive syntax for every source would make genuinely invalid module input appear complete.

Source-map source identities also contain information that is unrelated to the parser mode. main.cts?revision=2#view.tsx still names a .cts pathname; the fragment must not enable JSX. Likewise, main.mjs?view=main.cjs must remain an ES module and reject a top-level return. The URL pathname selects the mode, while the full URL remains the source identity published in the map, original-source record, and dependency edges.

The existing implementation supplies that pathname to the shared filename-aware parser. This PR checks the complete producer/consumer boundary around it rather than adding a second interpretation of filenames.

Change and scope

One parameterized test runs the same seven original-source cases against ordinary maps and indexed maps with a section offset. A real loopback HTTP server serves source-map JSON, and fetchWebSourceMaps receives schema-validated input with the explicit allowed origin.

The valid cases include .ts, .cts, .cjs, webpack:///src/main.ts, and .tsx. Each must retain its source text, report included without a limitation, and publish the expected static-import or require edge for ./dep.js. The test checks the edge's complete from_source identity and resolved dependency URL as well as the original-source and mapping identities.

Two deliberately invalid cases check the complementary rule: an angle-bracket assertion in .mts and a top-level return in .mjs remain partial, with an explicit parsing limitation. A query string that names a more permissive extension must not change those results.

The server is closed in finally, including active connections, so the fixture owns its network resources throughout the test. Both map formats run through the same production fetcher and parser path. The PR adds no source production changes, parsing options, request fields, dependencies, or browser operations.

Contract and boundary impact

Semantic owner and earliest changed stage: regression coverage at the HTTP source-map fetch/parse boundary; runtime ownership remains with the existing fetcher and parser.
Contribution scope and maintainer decision: focused tests for the existing browser source-map workflow; a new-capability scope decision is not applicable.
CLI and MCP/tool-catalog contract: unchanged. Assertions protect the existing completeness and module-edge output.
Provider, bridge, target-format, or platform compatibility: ordinary and indexed Source Map v3 documents served over loopback HTTP; no provider or bridge implementation changes.
Evidence, artifact, provenance, or reconstruction contract: the tests verify preservation of source text, full source URLs, mapping identities, and resolved module edges. They add no artifact format.
Target selection and cleanup: self-owned loopback fixture with an explicit origin, no live browser attachment; connections and the listener are released by the fixture.
Generated metadata, package, or installation impact: none; no build outputs or generated catalogs are committed.

Verification reads fixture data, serves it over a local HTTP connection, and runs the production fetcher/parser. It does not launch Chromium, inject page code, alter a caller's browser state, or contact an external application.

Evidence and regression coverage

The new file contains two parameterized test executions, each checking seven source variants. It complements the existing fetcher regression with indexed-map, webpack URL, TSX, strict module, and suffix-handling coverage. Assertions inspect returned production facts rather than stubbing the parser's answer.

The historical defect and its reproduction remain documented in #2954. The current test-only diff is validated against the implementation already present on main; it does not claim to introduce that implementation again. Running the fetcher without the original-path parser input would violate the completeness and dependency-edge assertions for valid TypeScript/CommonJS cases.

Validation performed

The branch normally merges main f75b1a416ba67b8421e7dbd248160afedb79581f and retains the maintainer's previous merge. Verification used Node.js 24.18.0 and npm 11.16.0 on Windows x64.

npm run test:focused -- tests/boundary/browser/webSourceMapParserModes.test.ts tests/boundary/browser/webSourceMapFetcher.test.ts tests/boundary/browser/optionalSourceMapRequire.test.ts tests/boundary/process/javascriptSourceModes.test.ts src/domain/reconstructionVerification.functionSources.test.ts — all 156 tests passed across five files, including the reconstruction suite discussed in review.
npm run check — passed typechecking, lint and resolved module boundaries, formatting, unused-code checks, package metadata checks, and architecture guards.
Configured pre-commit tasks through npx --no-install lint-staged --diff origin/main...HEAD — formatting and lint passed for the single changed file without further edits.
git diff --check origin/main...HEAD — passed; the working tree is clean and the maintainer's prior head remains an ancestor.

The regression uses actual HTTP fetching and the production parser on Windows x64. It does not establish an installed Chrome workflow or new direct CLI/MCP source-map coverage. No performance change is claimed for this test-only contribution.

The current-head GitHub CI required check has passed, including the source tests, build, static checks, and portable test suite.

Compatibility, safety, and release

There is no runtime behavior change or migration in the remaining diff. The tests protect the current behavior of the implementation on main. No dependency, installation, release metadata, or target authority changes are included.

Author readiness

Human owner: @dakjdakd

I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
I declare this PR ready for initial maintainer review.

Before merge

The remaining diff has one focused regression-coverage outcome.
The related issue and the implementation already on main are identified above.
Source identity, valid syntax, and intentionally incomplete syntax are checked together.
The final diff contains only the source-map test file.
Required checks pass for the current PR revision.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/2955 **Author:** [@dakjdakd](https://github.com/dakjdakd) **Created:** 10/13/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/source-map-parser-modes` --- ### 📝 Commits (3) - [`3fe4a20`](https://github.com/morluto/rea/commit/3fe4a2027f42057b82236344efa98a21f33b0a1a) fix(browser): preserve source-map original filename parser modes - [`22f89f5`](https://github.com/morluto/rea/commit/22f89f5e5d11edb0735480754918ea23e995d9e9) Merge remote-tracking branch 'origin/main' into HEAD - [`cb6cde0`](https://github.com/morluto/rea/commit/cb6cde064fc29462c71c6b18a7dbd25e51997d67) Merge remote-tracking branch 'origin/main' into fix/source-map-parser-modes ### 📊 Changes **1 file changed** (+116 additions, -0 deletions) <details> <summary>View changed files</summary> ➕ `tests/boundary/browser/webSourceMapParserModes.test.ts` (+116 -0) </details> ### 📄 Description ## Summary Add production-boundary regression coverage for filename-dependent parser modes in browser source maps. The source-map implementation from #2954 is already on main in [f6905719c](https://github.com/morluto/rea/commit/f6905719c3c80d829037e509d6e46129b131d893). The remaining change is one test file, `tests/boundary/browser/webSourceMapParserModes.test.ts`, which exercises that implementation through actual HTTP source-map fetching. The tests cover ordinary and indexed maps, TypeScript and CommonJS originals, `webpack://` source identities, and query/fragment suffixes that resemble other filename extensions. They verify both parser completeness and retained dependency edges while checking that the complete original source URL remains present in the returned evidence. Related issue: #2954, already resolved by the implementation on main. ## Problem and expected behavior An original filename is part of the parser's input semantics. For example, `main.ts` can contain `const value = <number>1`, while a CommonJS `main.cjs` can contain a top-level `return`. Parsing either without the appropriate filename mode can mark a valid original as incomplete and discard its dependency edges. Conversely, enabling permissive syntax for every source would make genuinely invalid module input appear complete. Source-map source identities also contain information that is unrelated to the parser mode. `main.cts?revision=2#view.tsx` still names a `.cts` pathname; the fragment must not enable JSX. Likewise, `main.mjs?view=main.cjs` must remain an ES module and reject a top-level return. The URL pathname selects the mode, while the full URL remains the source identity published in the map, original-source record, and dependency edges. The existing implementation supplies that pathname to the shared filename-aware parser. This PR checks the complete producer/consumer boundary around it rather than adding a second interpretation of filenames. ## Change and scope One parameterized test runs the same seven original-source cases against ordinary maps and indexed maps with a section offset. A real loopback HTTP server serves source-map JSON, and `fetchWebSourceMaps` receives schema-validated input with the explicit allowed origin. The valid cases include `.ts`, `.cts`, `.cjs`, `webpack:///src/main.ts`, and `.tsx`. Each must retain its source text, report `included` without a limitation, and publish the expected static-import or require edge for `./dep.js`. The test checks the edge's complete `from_source` identity and resolved dependency URL as well as the original-source and mapping identities. Two deliberately invalid cases check the complementary rule: an angle-bracket assertion in `.mts` and a top-level return in `.mjs` remain `partial`, with an explicit parsing limitation. A query string that names a more permissive extension must not change those results. The server is closed in `finally`, including active connections, so the fixture owns its network resources throughout the test. Both map formats run through the same production fetcher and parser path. The PR adds no source production changes, parsing options, request fields, dependencies, or browser operations. ## Contract and boundary impact - Semantic owner and earliest changed stage: regression coverage at the HTTP source-map fetch/parse boundary; runtime ownership remains with the existing fetcher and parser. - Contribution scope and maintainer decision: focused tests for the existing browser source-map workflow; a new-capability scope decision is not applicable. - CLI and MCP/tool-catalog contract: unchanged. Assertions protect the existing completeness and module-edge output. - Provider, bridge, target-format, or platform compatibility: ordinary and indexed Source Map v3 documents served over loopback HTTP; no provider or bridge implementation changes. - Evidence, artifact, provenance, or reconstruction contract: the tests verify preservation of source text, full source URLs, mapping identities, and resolved module edges. They add no artifact format. - Target selection and cleanup: self-owned loopback fixture with an explicit origin, no live browser attachment; connections and the listener are released by the fixture. - Generated metadata, package, or installation impact: none; no build outputs or generated catalogs are committed. Verification reads fixture data, serves it over a local HTTP connection, and runs the production fetcher/parser. It does not launch Chromium, inject page code, alter a caller's browser state, or contact an external application. ## Evidence and regression coverage The new file contains two parameterized test executions, each checking seven source variants. It complements the existing fetcher regression with indexed-map, webpack URL, TSX, strict module, and suffix-handling coverage. Assertions inspect returned production facts rather than stubbing the parser's answer. The historical defect and its reproduction remain documented in #2954. The current test-only diff is validated against the implementation already present on main; it does not claim to introduce that implementation again. Running the fetcher without the original-path parser input would violate the completeness and dependency-edge assertions for valid TypeScript/CommonJS cases. ## Validation performed The branch normally merges main `f75b1a416ba67b8421e7dbd248160afedb79581f` and retains the maintainer's previous merge. Verification used Node.js 24.18.0 and npm 11.16.0 on Windows x64. - `npm run test:focused -- tests/boundary/browser/webSourceMapParserModes.test.ts tests/boundary/browser/webSourceMapFetcher.test.ts tests/boundary/browser/optionalSourceMapRequire.test.ts tests/boundary/process/javascriptSourceModes.test.ts src/domain/reconstructionVerification.functionSources.test.ts` — all 156 tests passed across five files, including the reconstruction suite discussed in review. - `npm run check` — passed typechecking, lint and resolved module boundaries, formatting, unused-code checks, package metadata checks, and architecture guards. - Configured pre-commit tasks through `npx --no-install lint-staged --diff origin/main...HEAD` — formatting and lint passed for the single changed file without further edits. - `git diff --check origin/main...HEAD` — passed; the working tree is clean and the maintainer's prior head remains an ancestor. The regression uses actual HTTP fetching and the production parser on Windows x64. It does not establish an installed Chrome workflow or new direct CLI/MCP source-map coverage. No performance change is claimed for this test-only contribution. The current-head GitHub `CI required` check has passed, including the source tests, build, static checks, and portable test suite. ## Compatibility, safety, and release There is no runtime behavior change or migration in the remaining diff. The tests protect the current behavior of the implementation on main. No dependency, installation, release metadata, or target authority changes are included. ## Author readiness Human owner: @dakjdakd - [x] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. ## Before merge - [x] The remaining diff has one focused regression-coverage outcome. - [x] The related issue and the implementation already on main are identified above. - [x] Source identity, valid syntax, and intentionally incomplete syntax are checked together. - [x] The final diff contains only the source-map test file. - [ ] Required checks pass for the current PR revision. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:07:14 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-01 21:44:23 +00:00

[PR #5570] [MERGED] fix(signals): report rejected sigrok selectors as invalid input #6969

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

morluto/rea#5515

No description provided.

Rows
Columns