Skip to content

Revive or remove the last non-manual skipped tests - #8347

Merged
andrew-polk merged 1 commit into
masterfrom
reinstate-skipped-tests
Sep 11, 2026
Merged

Revive or remove the last non-manual skipped tests#8347
andrew-polk merged 1 commit into
masterfrom
reinstate-skipped-tests

Conversation

@andrew-polk

@andrew-polk andrew-polk commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Problem. After #8311 the nightly still reported six skipped C# tests and one skipped React component test. All seven were long-ignored tests whose reasons had gone stale, so the count kept hiding whether anything real was skipped.

Fix. Each is either revived or removed, by what the ignore reason turned out to mean:

  • HtmlDomTests, two plus-sign image-URL tests: the code was right and the tests' expectations were not. UrlPathString deliberately treats + as a literal plus and encodes it as %2b (BL-3259). One expectation changed; both run again.
  • XmlHtmlConverterTests, em next to strong: the test built its DOM without PreserveWhitespace, a shape production never saves, so the indenting writer split the two inline elements. Every real book is loaded with PreserveWhitespace on (BookStorage, BL-2484). The test now does the same and passes. Confirmed in a live Bloom that the app never had the bug, which is why BL-2558 was unreproducible; BL-16858 records that.
  • Deleted: BookTests CanDelete_TemplateBook_False (deletability has not depended on templateness since BL-2678) and two BookStarterTests Vaccinations cover tests (cover image and topic are filled in by Book, not BookStarter).
  • ToolboxRoot component test: the fixme is a real test again. The React header does render a per-tool icon and a subscription badge for Canvas, Motion and Music. Both elements now carry test ids and the test asserts on those instead of computed styles.

Expected on the next nightly: C# and React component skips both go to 0.

🤖 Generated with Claude Code

Devin review


This change is Reviewable

@andrew-polk

Copy link
Copy Markdown
Contributor Author

[Claude Fable] Consulted Devin on 2026-09-11 00:19 UTC up to commit 90a3c0f. No bugs. Two Investigate flags on the revived toolbox header test, mirrored as review threads and both fixed in 8725736. No informational flags. Re-review of 8725736 follows.

@andrew-polk

Copy link
Copy Markdown
Contributor Author

[Claude Fable] Consulted Devin on 2026-09-11 00:24 UTC up to commit 8725736. No bugs. One new Investigate flag (the icon test read the inline style), mirrored as a review thread and fixed in 1324dee; the other two flags were repeats already fixed in the reviewed commit. No informational flags. Re-review of 1324dee follows.

@andrew-polk

Copy link
Copy Markdown
Contributor Author

[Claude Fable] Consulted Devin on 2026-09-11 00:26 UTC up to commit 1324dee. Re-review clean: no bugs, and its three Investigate flags are repeats of findings already fixed in the reviewed commits (the icon test now reads data-icon-src, checks for a real image, and documents its wait). No informational flags.

After #8311 the nightly still reported six skipped C# tests and one skipped
React component test, all long-ignored with stale reasons. Each is revived
or removed by what the reason turned out to mean.

- HtmlDomTests: the two plus-sign GetImageElementUrl tests run again. The code
  was right; UrlPathString treats '+' as a literal plus and encodes it as %2b
  (BL-3259), so one expectation is corrected.
- XmlHtmlConverterTests: the em-next-to-strong test now sets PreserveWhitespace
  before loading, as BookStorage does for every real book (BL-2484), and passes.
  A live Bloom confirmed the app never had the bug; BL-16858 records why
  BL-2558 was unreproducible.
- Deleted BookTests.CanDelete_TemplateBook_False (deletability has not depended
  on templateness since BL-2678) and the two BookStarterTests Vaccinations cover
  tests (cover image and topic are Book's job, not BookStarter's).
- ToolboxRoot component test: the fixme is a real test. The header icon span
  carries a test id and data-icon-src, the subscription badge carries a test id,
  and the test asserts on those rather than on computed styles.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@andrew-polk
andrew-polk force-pushed the reinstate-skipped-tests branch from 1324dee to af3b370 Compare September 11, 2026 04:09
@andrew-polk
andrew-polk marked this pull request as ready for review September 11, 2026 04:09
@andrew-polk
andrew-polk merged commit 8bd12db into master Sep 11, 2026
1 check passed
@andrew-polk
andrew-polk deleted the reinstate-skipped-tests branch September 11, 2026 04:10
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