Skip to content

Test CTabItem rendering#3295

Draft
basilevs wants to merge 1 commit into
eclipse-platform:masterfrom
basilevs:test_ctabfolder
Draft

Test CTabItem rendering#3295
basilevs wants to merge 1 commit into
eclipse-platform:masterfrom
basilevs:test_ctabfolder

Conversation

@basilevs
Copy link
Copy Markdown
Contributor

@basilevs basilevs commented May 10, 2026 •
edited
Loading

Pixel color verification for CTabItem's tabs.

A regression test for #3260 where tabs fail to render if
NSView.setClipsToBounds() is omited before scheduling redraw.

This test fails to reproduce the problem, but is useful as rendering is
not tested in this suite yet.

@basilevs basilevs force-pushed the test_ctabfolder branch 2 times, most recently from ce76b95 to 3ace3b9 Compare May 10, 2026 18:21
merks
merks reviewed May 10, 2026
View reviewed changes
Comment thread ...wt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_custom_CTabFolder.java Outdated
/* custom */
protected CTabFolder ctabFolder;
private Color RED;
private Color BLUE;
Copy link
Copy Markdown
Contributor

@merks merks May 10, 2026

Choose a reason for hiding this comment

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

It seems odd to use an all caps name for something that’s not a constant.

Copy link
Copy Markdown
Contributor Author

@basilevs basilevs May 10, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Colors in SWT depend on Display, which I access via Shell, which is initialized after test starts. I could try to use Display.getCurrent() but this would be riskier (Display can be recreated). Therefore these are constants effectively but not in practice. Should I make them lowercase?

Copy link
Copy Markdown
Contributor

@merks merks May 10, 2026

Choose a reason for hiding this comment

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

Actually there are now Color constructors without a device and colors need not be disposed so they don’t really depend on a display/device.

When I see an all caps name, I expect it to static final, so yes, I’d prefer the name to match general expectations…

Copy link
Copy Markdown
Contributor Author

@basilevs basilevs May 10, 2026

Choose a reason for hiding this comment

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

I've made constants final using new constructors.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 10, 2026 •
edited
Loading

Test Results

  175 files   -  7    175 suites   - 7   27m 59s ⏱️ + 1m 10s
4 708 tests  - 15  4 682 ✅  - 18   22 💤  - 1   4 ❌ + 4 
6 727 runs   - 85  6 554 ✅  - 95  157 💤  - 6  16 ❌ +16 

For more details on these failures, see this check.

Results for commit 0a65341. ± Comparison against base commit a4a774c.

This pull request removes 19 and adds 4 tests. Note that renamed tests count towards both. 
org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_HeuristicASCII_dollarSign
org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_HeuristicASCII_emptyString
org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_HeuristicASCII_letterA
org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_HeuristicASCII_letters
org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_HeuristicUTF16LE_null
org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_HeuristicUTF16_AsciiLetters
org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_HeuristicUTF16_Asciiletter
org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_HeuristicUTF16_LotsOfLetters
org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_HeuristicUTF16_letter
org.eclipse.swt.tests.gtk.Test_GtkConverter ‑ test_HeuristicUTF16_letters
…

org.eclipse.swt.tests.junit.Test_org_eclipse_swt_custom_CTabFolder ‑ test_tabsAreRendered(boolean, int)[1]
org.eclipse.swt.tests.junit.Test_org_eclipse_swt_custom_CTabFolder ‑ test_tabsAreRendered(boolean, int)[2]
org.eclipse.swt.tests.junit.Test_org_eclipse_swt_custom_CTabFolder ‑ test_tabsAreRendered(boolean, int)[3]
org.eclipse.swt.tests.junit.Test_org_eclipse_swt_custom_CTabFolder ‑ test_tabsAreRendered(boolean, int)[4]

♻️ This comment has been updated with latest results.

@basilevs
Copy link
Copy Markdown
Contributor Author

basilevs commented May 10, 2026 •
edited
Loading

Windows CI is passing, Linux and MacOS are not. The test is passing on my MacOS. Configuration issue? The test seems trivial - it should work, as long as graphical context is available.

How do I get screenshots? They are saved to /tmp and are not archived.

@basilevs basilevs mentioned this pull request May 10, 2026
Draft
@basilevs basilevs marked this pull request as draft May 10, 2026 20:16
@basilevs
Ensure that CTabItems are rendered
0a65341 
A regression test for eclipse-platform#3260 where tabs fail to render if
`NSView.setClipsToBounds()` is omited before scheduling redraw.

This test fails to reproduce the problem, but is useful as rendering is
not tested at all in this suite.
@basilevs basilevs marked this pull request as ready for review May 10, 2026 20:20
@basilevs basilevs marked this pull request as draft May 10, 2026 20:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@merks merks merks left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@basilevs @merks