Runners: set FullScreen to runners main test shell #1316
Conversation
WalkthroughAdds a SWT UI-thread init that obtains the active workbench shell, maximizes it, and attempts to set fullscreen (exceptions ignored) before proceeding with existing Welcome view and UI automation steps. No changes to method signatures or return values. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant TestRunner as Test Runner
participant SWT as SWT UI Thread
participant WB as Workbench
participant Shell as Shell
TestRunner->>SWT: Display.syncExec(init)
SWT->>WB: PlatformUI.getWorkbench().getActiveWorkbenchWindow()
WB-->>SWT: ActiveWorkbenchWindow
SWT->>Shell: getShell()
SWT->>Shell: setMaximized(true)
alt fullscreen supported
SWT->>Shell: try setFullScreen(true)
else unsupported/error
Note right of Shell: exception caught and ignored
end
SWT-->>TestRunner: UI initialized
TestRunner->>WB: continue to open Welcome view and run UI automation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🔇 Additional comments (1)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/com.espressif.idf.ui.test/src/com/espressif/idf/ui/test/operations/EnvSetupOperations.java(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build_macos
| Display.getDefault().syncExec(() -> { | ||
| Shell shell = PlatformUI.getWorkbench().getActiveWorkbenchWindow().getShell(); | ||
| // Maximize (Windows/Linux/macOS windowed) | ||
| shell.setMaximized(true); | ||
| if (Platform.OS_WIN32.equals(Platform.getOS()) || Platform.OS_LINUX.equals(Platform.getOS()) || Platform.OS_MACOSX.equals(Platform.getOS())) { | ||
| try { shell.setFullScreen(true); } catch (Throwable ignore) {} | ||
| } | ||
| }); |
There was a problem hiding this comment.
Guard against missing workbench window before toggling fullscreen
PlatformUI.getWorkbench().getActiveWorkbenchWindow() can be null during startup (or if the window is closing), which will crash this setup with an NPE before the tests even get a chance to run. Please bail out gracefully when the display/window/shell aren’t ready yet.
Apply this diff to harden the block:
- Display.getDefault().syncExec(() -> {
- Shell shell = PlatformUI.getWorkbench().getActiveWorkbenchWindow().getShell();
- // Maximize (Windows/Linux/macOS windowed)
- shell.setMaximized(true);
- if (Platform.OS_WIN32.equals(Platform.getOS()) || Platform.OS_LINUX.equals(Platform.getOS()) || Platform.OS_MACOSX.equals(Platform.getOS())) {
- try { shell.setFullScreen(true); } catch (Throwable ignore) {}
- }
- });
+ Display display = Display.getDefault();
+ if (display == null)
+ return;
+ display.syncExec(() -> {
+ IWorkbenchWindow window = PlatformUI.getWorkbench().getActiveWorkbenchWindow();
+ if (window == null)
+ return;
+ Shell shell = window.getShell();
+ if (shell == null || shell.isDisposed())
+ return;
+ // Maximize (Windows/Linux/macOS windowed)
+ shell.setMaximized(true);
+ if (Platform.OS_WIN32.equals(Platform.getOS()) || Platform.OS_LINUX.equals(Platform.getOS())
+ || Platform.OS_MACOSX.equals(Platform.getOS())) {
+ try {
+ shell.setFullScreen(true);
+ }
+ catch (Throwable ignore) {
+ }
+ }
+ });Add the missing import alongside the other Eclipse UI imports:
import org.eclipse.swtbot.swt.finder.widgets.SWTBotShell;
+import org.eclipse.ui.IWorkbenchWindow;
import org.eclipse.ui.PlatformUI;
| Shell shell = PlatformUI.getWorkbench().getActiveWorkbenchWindow().getShell(); | ||
| // Maximize (Windows/Linux/macOS windowed) | ||
| shell.setMaximized(true); | ||
| if (Platform.OS_WIN32.equals(Platform.getOS()) || Platform.OS_LINUX.equals(Platform.getOS()) || Platform.OS_MACOSX.equals(Platform.getOS())) { |
There was a problem hiding this comment.
We can avoid this since there are all OS from the Platform mentioned in the condition
Description
Some SWT tests were constantly failing due to incorrect main shell size.
Type of change
Please delete options that are not relevant.
Checklist
Summary by CodeRabbit