Re-enable and stabilize previously-disabled tests#3377
Merged
Conversation
Several test suites were disabled (xit/xdescribe) due to flakiness or environment dependence. Re-enable them and fix the underlying issues so they pass deterministically regardless of spec order: - selectionConverterTest: spy on the deep createRange module did not intercept the barrel import; exercise the real createRange with a document-attached image instead. - updateRotateHandleTest: the spy targeted rotateHandle while the code reads rotateCenter; point it at the right element and feed each "hidden" case the intended edge distance (and correct an impossible expected handle value). - applyChangeTest: replace brittle exact PNG data-URL comparisons with IHDR-dimension checks (encoder-independent), and create a fresh contentModelImage per test to remove order-dependent editingInfo state. - tableEditorTest: give the editor a deterministic size so the table is never flush against the scroll-container edge, which otherwise made inserter visibility depend on leftover document.body layout. - tableResizerTest: recreate the content model table per test (onDragging mutates it in place) and gate verifyCellRectChange on growth only so an incidental cross-axis border-box shift does not flip the assertion. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
JiuqingSong
requested review from
BryanValverdeU,
Copilot,
flyingbee2012,
ianeli1,
juliaroldi and
vinguyen12
Contributor
There was a problem hiding this comment.
Pull request overview
This PR re-enables previously disabled/flaky unit tests across multiple RoosterJS packages by removing order-dependent shared state, avoiding platform-dependent assertions (e.g., canvas/base64 output), and making layout/selection setup deterministic so results are consistent across OS/browser variations.
Changes:
- Re-enabled multiple
xit/xdescribesuites and addressed root flakiness causes (shared mutable models, viewport/layout dependence). - Updated image-edit tests to avoid non-portable
canvas.toDataURL()byte-for-byte comparisons by asserting stable PNG IHDR dimensions instead. - Stabilized selection- and paste-related tests by relying on real DOM nodes / deterministic selection placement.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/roosterjs-editor-adapter/test/editor/utils/selectionConverterTest.ts | Re-enables image-selection coverage by asserting against a real DOM <img> and the produced Range (avoids brittle spying). |
| packages/roosterjs-content-model-plugins/test/tableEdit/tableResizerTest.ts | Re-enables resize dragging suite and removes cross-test state leakage by recreating the table model per test. |
| packages/roosterjs-content-model-plugins/test/tableEdit/tableEditorTest.ts | Re-enables disableFeatures tests by enforcing deterministic editor sizing/padding to avoid order-dependent layout. |
| packages/roosterjs-content-model-plugins/test/paste/e2e/testUtils.ts | Simplifies expectEqual to a strict model equality assertion after normalization (clone + JSON roundtrip). |
| packages/roosterjs-content-model-plugins/test/imageEdit/utils/applyChangeTest.ts | Re-enables Linux-disabled suite by eliminating shared module state and replacing non-portable base64 comparisons with PNG-dimension checks. |
| packages/roosterjs-content-model-plugins/test/imageEdit/Rotator/updateRotateHandleTest.ts | Re-enables rotate-handle positioning tests with corrected spying target and updated expected geometry. |
| packages/roosterjs-content-model-core/test/command/paste/pasteTest.ts | Re-enables “Second paste” by setting a deterministic collapsed selection before reading the content model. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
juliaroldi
approved these changes
Jun 18, 2026
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.
Summary
A number of unit tests were previously disabled (
xit/xdescribe) because they were flaky or platform-dependent. This PR re-enables them and fixes the underlying causes so they pass deterministically regardless of OS, browser, and spec execution order.Changes
roosterjs-content-model-core— pastepasteTest.ts: Re-enabled the'Second paste'test.roosterjs-content-model-plugins— image editupdateRotateHandleTest.ts: Re-enabled the rotate-handle positioning suite. The spy was attached to the wrong element (rotateHandleinstead ofrotateCenter), and the test geometry/expected values were corrected so the assertions reflect the actual layout math. These tests were previously marked "not consistent".applyChangeTest.ts: Re-enabled the suite (was disabled because it "fails on Linux").contentModelImagewas shared module-level state;applyChangepersists/clearseditingInfoon it, so a stale rotation/crop from a prior test could misclassify the next one. It is now recreated fresh inbeforeEach.canvas.toDataURL()output varies by browser/OS/Chrome version (compression, ancillary chunks), so exact base64 comparison wasn't portable — the original reason the tests were Linux-disabled. Assertions now verify the data URL is a PNG and compare the IHDR width/height, which is stable across encoders by the PNG spec. The one case wherenewSrccomes verbatim from the mocked event handler still uses exact comparison (documented inline).roosterjs-content-model-plugins— table edittableEditorTest.ts: Re-enabled thedisableFeaturessuite. The editor div now gets a deterministic size with padding on every side, so the table is never flush against the scroll-container edge. Previously, whether the cell inserter was suppressed (isOutsideTop/isOutsideBottom) depended on leftoverdocument.bodylayout from earlier specs, making the test order-dependent.tableResizerTest.ts: Re-enabled theonDraggingresize suite.cmTableis now recreated inbeforeEach—onDraggingmutates it in place, leaking resized widths/heights into later tests.onDraggingrewrites both width and height of every cell (switching to border-box), so resizing one axis incidentally shifts the other by the border width. The assertion now checks only the axis being resized (gated by growth sign) so small cross-axis shifts don't flip the expected direction.roosterjs-editor-adapter— selection conversionselectionConverterTest.ts: Re-enabled the'image selection'test.createRangeis re-exported through theroosterjs-editor-dombarrel via a getter, which webpack won't let us spy on. The test now uses a real<img>node and asserts on the resultingRangeinstead of mockingcreateRange.Testing
yarn test:fast