Skip to content

Run the StyleEditor tests in the browser project; un-skip the two that need it (BL-16925) - #8411

Merged
andrew-polk merged 1 commit into
masterfrom
BL-16925-styleeditor-tests
Sep 25, 2026
Merged

andrew-polk merged 1 commit into
masterfrom
BL-16925-styleeditor-tests

Conversation

@andrew-polk

@andrew-polk andrew-polk commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Problem. After #8408, the nightly still skipped 2 Vitest tests, both in the StyleEditor spec: "…MakeBigger replaces the existing rule" and "…ChangeSizeAbsolute replaces the existing rule". They were skipped in the Vitest move because they read computed styles (getComputedStyle), which jsdom doesn't really support. The same reason had kept the overflow tests off.

Fix. Rename StyleEditorSpec.ts to StyleEditor.browser.spec.ts, so the whole file runs in the browser Vitest project (headless Edge) added in #8408, and un-skip the two tests. All 23 pass there. The two now-obsolete "skipped because jsdom" comments are gone, and one line at the top says why the file runs in the browser.

Result: pnpm test runs 969 tests with none skipped (the last nightly had 967 passed, 2 skipped).

While these tests run, StyleEditor asks Bloom for fonts/metadata. With no Bloom to answer, the request fails and a "*****Got error trying report preliminaryError" line is logged. It's harmless: under jsdom the same failure is filtered out by vitest.setup.ts.

Ref: https://issues.bloomlibrary.org/youtrack/issue/BL-16925

🤖 Generated with Claude Code

Devin review


This change is Reviewable

…that need it (BL-16925)

Two StyleEditor tests had been skipped since the move to Vitest because they read computed styles, which jsdom doesn't support. The file is now StyleEditor.browser.spec.ts, so all 23 of its tests run in headless Edge, and those two run again. That leaves no skipped tests in the Vitest suite.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@andrew-polk

Copy link
Copy Markdown
Contributor Author

[Claude Opus 5.5] Consulted Devin on 2026-09-25 up to commit 62a8ad0. Clean: no bugs and no flags.

@greptile-apps

greptile-apps Bot commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Retrigger

[Low risk] Moves style editor tests to browser test runner.

The PR appears safe to merge, though the browser test should avoid its known failed API requests.

Summary

The PR moves the StyleEditor spec from jsdom to headless Edge and enables two tests that require computed styles.

  • The browser project collects the renamed file.
  • One existing test now produces avoidable failed API requests and error-reporting noise.

Reviews (1) · Last reviewed commit: "Run the StyleEditor tests in the browser..."

@andrew-polk

Copy link
Copy Markdown
Contributor Author

[Claude Opus 5.5] Re Greptile's note about avoidable failed API requests: two tests reach UpdateControlsToReflectAppliedStyle, which asks Bloom for fonts/metadata. With no Bloom in the test run, that request fails, and the error reporter logs a failed attempt to report it. The same request failed under jsdom too; vitest.setup.ts just filtered the message out. So the tests behave exactly as before, and none depends on the answer. Stubbing it didn't work: vi.mock of utils/bloomApi doesn't take effect, because the browser setup file already imports that module through the localization manager. The remaining options are changing the tests' inputs so they skip the font lookup, or changing the production error reporter to detect browser tests. Neither seems worth it for two harmless log lines, so I'm leaving it.

@andrew-polk
andrew-polk marked this pull request as ready for review September 25, 2026 21:45
@andrew-polk
andrew-polk merged commit 4938d21 into master Sep 25, 2026
2 checks passed
@andrew-polk
andrew-polk deleted the BL-16925-styleeditor-tests branch September 25, 2026 21:45
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.

1 participant