Conversation
This PR inits the setup for Playwright tests and adds tests that cover the most important functionality of the app.
This comment was marked as off-topic.
This comment was marked as off-topic.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
tests/_support/office-mock.ts (1)
16-16: ⚡ Quick winMake Office route matching resilient to URL variants.
Using an exact URL can miss query-string/cdn variations of
office.js. Prefer a wildcard/glob pattern.Suggested patch
- await page.route("https://appsforoffice.microsoft.com/lib/1/hosted/office.js", async (route) => { + await page.route("**/lib/1/hosted/office.js*", async (route) => { await route.fulfill({ contentType: "application/javascript", body: await compileOfficeMock() }); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/_support/office-mock.ts` at line 16, The page.route call is matching the exact office.js URL and will miss variants with query strings or CDN paths; update the page.route invocation in tests/_support/office-mock.ts (the page.route(...) handler and its async route callback) to use a resilient matcher (a glob or RegExp) that matches any path ending with office.js (and optional query) on appsforoffice.microsoft.com so the route will intercept variants like query-string or CDN-modified URLs.tests/_support/transpile-browser-mock.ts (1)
11-19: ⚡ Quick winFail fast on TypeScript transpilation diagnostics.
transpileModulediagnostics are ignored, which can hide broken mock sources until browser runtime. Throw on diagnostics during compilation.Suggested patch
- const output = ts.transpileModule(source, { + const result = ts.transpileModule(source, { compilerOptions: { module: ts.ModuleKind.ES2022, target: ts.ScriptTarget.ES2022, sourceMap: false, }, fileName, - }).outputText; + reportDiagnostics: true, + }); + + if (result.diagnostics?.length) { + throw new Error( + ts.formatDiagnosticsWithColorAndContext(result.diagnostics, { + getCurrentDirectory: () => process.cwd(), + getCanonicalFileName: (f) => f, + getNewLine: () => "\n", + }), + ); + } + + const output = result.outputText;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/_support/transpile-browser-mock.ts` around lines 11 - 19, The current call to ts.transpileModule discards diagnostics; change the code to capture the result (e.g., const result = ts.transpileModule(...)), inspect result.diagnostics, and if any diagnostics exist throw an Error (including fileName and formatted diagnostic messages) instead of returning outputText blindly; then use result.outputText as the output variable. This ensures ts.transpileModule, result.diagnostics, outputText and fileName are used to fail fast on TypeScript transpilation errors.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/test.yml:
- Line 16: The workflow uses floating action refs (e.g., the uses line
referencing actions/checkout@v6) which should be pinned to full commit SHAs;
update every uses: entry (such as the actions/checkout reference) to the
corresponding full commit SHA instead of the tag, optionally leaving the
human-readable tag as an end-of-line comment (e.g., keep “@v6” in a comment) for
readability, and ensure each replacement is the exact commit SHA for that action
to harden the CI supply chain.
In `@tests/_support/browser-mocks/typst-state.ts`:
- Around line 27-29: The readiness predicate typstMockReady currently returns
true only when typstMockState.rendererInitOptions.length === 1 which can hang if
initialization runs more than once; change the predicate to check for length >=
1 (or replace with an explicit boolean latch) so waitUntilReady() becomes true
as soon as at least one init occurs—update the function typstMockReady to use >=
1 and ensure any tests relying on a single-init assumption are adjusted
accordingly.
---
Nitpick comments:
In `@tests/_support/office-mock.ts`:
- Line 16: The page.route call is matching the exact office.js URL and will miss
variants with query strings or CDN paths; update the page.route invocation in
tests/_support/office-mock.ts (the page.route(...) handler and its async route
callback) to use a resilient matcher (a glob or RegExp) that matches any path
ending with office.js (and optional query) on appsforoffice.microsoft.com so the
route will intercept variants like query-string or CDN-modified URLs.
In `@tests/_support/transpile-browser-mock.ts`:
- Around line 11-19: The current call to ts.transpileModule discards
diagnostics; change the code to capture the result (e.g., const result =
ts.transpileModule(...)), inspect result.diagnostics, and if any diagnostics
exist throw an Error (including fileName and formatted diagnostic messages)
instead of returning outputText blindly; then use result.outputText as the
output variable. This ensures ts.transpileModule, result.diagnostics, outputText
and fileName are used to fail fast on TypeScript transpilation errors.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 31b74c82-43c2-46be-a132-54455bdfcc1b
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (27)
.config/vite.config.js.github/workflows/test.yml.gitignore.vscode/extensions.jsonDEV.mdpackage.jsonplaywright.config.tstests/_support/browser-mocks/typst-memory-access-model.tstests/_support/browser-mocks/typst-options.tstests/_support/browser-mocks/typst-package-registry.tstests/_support/browser-mocks/typst-state-module.d.tstests/_support/browser-mocks/typst-state.tstests/_support/browser-mocks/typst-wasm-url.tstests/_support/browser-mocks/typst.tstests/_support/fixtures.tstests/_support/office-mock.tstests/_support/office/adapter.tstests/_support/office/document-model.tstests/_support/office/install.tstests/_support/office/office-primitives.tstests/_support/office/types.tstests/_support/transpile-browser-mock.tstests/_support/typst-mock.tstests/pages/powerpoint-page.tstests/powerpoint.spec.tstests/preview.spec.tstsconfig.json
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.
No description provided.