Skip to content

Regression tests for #223, #360 and #1485 - #2293

Open
skylartaylor wants to merge 4 commits into
mainfrom
test/backfill-regression-tests
Open

skylartaylor wants to merge 4 commits into
mainfrom
test/backfill-regression-tests

Conversation

@skylartaylor

@skylartaylor skylartaylor commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Three fixes merged without a regression test. This adds one for each. Every test fails when its fix is reverted on current main and passes with the fix.

fix test fails without the fix because
#223: the title bar's Save drops the cell being typed file_menu::the_title_bar_save_button_saves_the_cell_being_typed the editor stays open and the saved file lacks the typed value
#360 (#342): Watch Window overflowed the Formulas ribbon interaction::the_formulas_ribbon_keeps_watch_window_inside_the_ribbon Watch Window's label ends at y=170, below the ribbon's last label row (y=155)
#1485: the PivotTable field-list scrollbar sat beside the field names pivot_pane::the_field_list_scrollbar_sits_at_the_pane_edge with 14 short field names the scrollbar sits at x≈941, not at the pane edge (~1190)

Test-only: no production code changes.

The ribbon's icon, big and small buttons have no accessible name, because their text is painted. So the tests find them another way:

Giving those buttons accessible names would help screen readers and simplify tests like these. I tried it, but it makes get_by_label("Delete" / "Cancel" / "Save" / "Copy") ambiguous in 8 existing tests, so it belongs in its own PR.

These came out of an audit of fix PRs merged without tests. The other gridcraft rows needed nothing new:

cargo xtask ci: all 8 steps pass.

Summary by CodeRabbit

  • Tests
    • Added coverage confirming that saving from the title bar preserves an in-progress cell edit in the workbook.
    • Added checks for label alignment in the Formulas ribbon and scrollbar placement in the pivot field list.
    • These regression tests help detect issues with saving edits and displaying these interface elements.

#223 made the title bar's Save button (and the Save Changes dialog's) commit the cell edit before saving, without a test for the button. The test types into A1, clicks Save, and reads the saved file back; with the commit_edit call removed it fails because the file lacks the value.
#360 (#342) moved Watch Window out of a three-row column it overflowed, without a test. The test checks Watch Window's painted label ends no lower than the ribbon's last label row; as the fourth small button it ends 15 px below.
… edge

#1485 let the field list take the pane's width so its scrollbar sits at the pane edge, without a test. With 14 short field names the scrollbar otherwise sits beside them, about 250 px from the edge.
@coderabbitai

coderabbitai Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
📥 Commits

Reviewing files that changed from the base of the PR and between bc0cd89 and 0ee8c0d.

📒 Files selected for processing: 3
  • crates/ui-egui/tests/it/file_menu.rs
  • crates/ui-egui/tests/it/interaction.rs
  • crates/ui-egui/tests/it/pivot_pane.rs

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

📝 Walkthrough

This pull request adds three UI regression tests. They check whether title-bar Save commits an active cell draft, whether two Formulas ribbon labels align vertically, and whether the pivot field-list scrollbar sits near the pane’s right edge.

Changes: Title-Bar Save

Layer / File(s) Summary
Save an active cell draft
crates/ui-egui/tests/it/file_menu.rs
A helper locates a title-bar button by its tooltip. The test clicks Save while a cell draft is active, then checks the saved value in A1. It removes the temporary directory.

Formulas Ribbon Alignment

Layer / File(s) Summary
Check Formulas ribbon label alignment
crates/ui-egui/tests/it/interaction.rs
The test collects painted text bounds and checks that the Watch Window label ends no more than 0.5 pixels below the Calculation Options label.

Pivot Field-List Scrollbar

Layer / File(s) Summary
Check pivot field-list scrollbar placement
crates/ui-egui/tests/it/pivot_pane.rs
A helper flattens nested shapes. The test renders a pivot table with 14 fields, reveals the floating scrollbar, and checks that its right edge is within 20 pixels of the pane’s right edge.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Other

Suggested reviewers: aiongg

Merge Risk: ⚪ Minimal · up to 0ee8c

The new tests cover the three intended UI behaviors. No issue identified here needs resolution before merge, subject to normal checks.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 72.73% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 22 functions across 6 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely identifies the main change: adding regression tests for issues #223, #360, and #1485.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@sonarqubecloud

sonarqubecloud Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

@FredApps FredApps left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving at 0ee8c0d. Tests only. I read the whole diff: a title-bar Save test that checks the draft in the cell being typed is committed and saved (found by hovering for the "Save (" tooltip), a Formulas ribbon test that Watch Window's label does not end below the ribbon's last label row, and a pivot field list test that its scrollbar sits at the pane edge. cargo xtask ci, both SonarCloud checks pass at this head.

This branch has not been deployed

No deployments
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.

2 participants