feat(java): ✨ adding windows support #1068
Conversation
WalkthroughAdds Windows desktop support across core Java: new WindowsManager, enum/platform additions, Locator update, and configuration fields. Introduces a Windows TestNG suite and GitHub Actions job to run Windows desktop tests and include their reports. Updates VSCode setting, adds Nx guidance doc, and bumps various package versions. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor QA as Tester
participant DM as DriverManager
participant WM as WindowsManager
participant SS as Session (Service URL)
participant AD as Appium WindowsDriver
QA->>DM: createDriver(platform=WINDOWS)
DM->>DM: detect platform & start desktop Appium path
DM->>WM: new WindowsManager(session)
DM->>WM: setupDriver()
activate WM
WM->>WM: setCommonOptions(machine settings)
WM->>WM: setApplicationOptions(app settings)
WM->>WM: setTimeoutOptions(timeouts)
WM->>SS: getServiceUrl()
WM->>AD: createDriver(url, WindowsOptions)
alt session not created
WM->>WM: handleAndThrow(SESSION_NOT_STARTED)
else success
WM-->>DM: WindowsDriver
end
deactivate WM
DM-->>QA: Driver instance
note over DM,WM: WINDOWS handled as first-class desktop platform
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
website/package.json (1)
49-49: Align pnpm version with the root to prevent “packageManager” conflicts.Root uses [email protected]; this package declares [email protected]. Mismatch causes lockfile churn and CI/env drift.
Pick one:
- Align here to 10.14.0 (simple), or
- Remove this field and rely on the root.
- "packageManager": "[email protected]" + "packageManager": "[email protected]"core-java/src/main/java/io/github/boykaframework/manager/DriverSession.java (1)
144-153: Don’t return null for WINDOWS; align with MAC desktop language retrievalReturning null risks NPEs for consumers expecting a LanguageSetting. Windows is a desktop platform like Mac; reuse the desktop machine language.
public LanguageSetting getLanguageSettings () { return switch (getPlatformType ()) { case ANDROID, IOS -> getMobileSetting ().getDevice () .getLanguage (); - case MAC -> getDesktopSetting ().getMachine () - .getLanguage (); + case MAC, WINDOWS -> getDesktopSetting ().getMachine () + .getLanguage (); case API -> getApiSetting ().getLanguage (); case WEB -> getWebSetting ().getLanguage (); - case WINDOWS -> null; }; }I can add a focused unit test to cover WINDOWS to prevent regressions.
core-java/src/main/java/io/github/boykaframework/manager/DriverManager.java (1)
79-85: Possible NPE when platform is not mobileCondition accesses session.getMobileSetting() for non-MAC platforms, which can be null for WEB/WINDOWS.
Apply:
- if (this.platformType == PlatformType.WEB || (this.platformType != MAC && session.getMobileSetting () - .getDevice () - .getApplication () - .getType () == ApplicationType.WEB)) { + if (this.platformType == PlatformType.WEB + || ((this.platformType == ANDROID || this.platformType == IOS) + && session.getMobileSetting () + .getDevice () + .getApplication () + .getType () == ApplicationType.WEB)) {
🧹 Nitpick comments (16)
website/package.json (1)
18-22: Pin all @docusaurus/ to the same version to avoid subtle mismatches.*Core is pinned to 3.8.1, others are ^3.8.0. Align to 3.8.1 for determinism.
- "@docusaurus/plugin-google-tag-manager": "^3.8.0", + "@docusaurus/plugin-google-tag-manager": "3.8.1", - "@docusaurus/theme-classic": "^3.8.0", + "@docusaurus/theme-classic": "3.8.1",.vscode/settings.json (1)
10-11: Ensure VS Code actually uses the workspace TypeScript.Set TS SDK path for deterministic tsserver behavior in the monorepo.
"typescript.enablePromptUseWorkspaceTsdk": true, + "typescript.tsdk": "node_modules/typescript/lib", "nxConsole.generateAiAgentRules": trueAlso confirm your NX Console extension version recognizes
nxConsole.generateAiAgentRules.core-java/src/main/java/io/github/boykaframework/config/ui/desktop/machine/MachineSetting.java (1)
39-39: Clarify/rename ‘experimental’ to be driver-specific.Ambiguous across platforms. Make intent explicit.
- private boolean experimental; + /** Enables experimental WebDriver mode for Windows driver. */ + private boolean experimentalWebDriver;core-java/src/main/java/io/github/boykaframework/config/ui/desktop/machine/ApplicationSetting.java (1)
29-35: Align field names with Appium Windows driver capabilities and document unitsReplace Java fields to match the Appium Windows driver’s capability names and clearly document their expected units:
- private String arguments; + /** appium:appArguments — command-line arguments (space-delimited string). */ + private String appArguments; - private int launchTimeout; + /** appium:createSessionTimeout — timeout in milliseconds to wait for session start. */ + private int createSessionTimeout; - private String path; + /** appium:app — absolute path to the app executable or bundle. */ + private String app; - private String topLevelWindow; + /** appium:appTopLevelWindow — hex window handle (e.g., 0x00…) to attach to an existing window. */ + private String appTopLevelWindow; - private String workingDir; + /** appium:appWorkingDir — working directory for the app process. */ + private String appWorkingDir;Enforce in WindowsManager that only one of
apporappTopLevelWindowcan be set at a time..github/instructions/nx.instructions.md (3)
13-13: Tone nit: replace double exclamationUse a single period for formal docs.
- - For questions around nx configuration, best practices or if you're unsure, use the nx_docs tool to get relevant, up-to-date docs!! Always use this instead of assuming things about nx configuration + - For questions around nx configuration, best practices or if you're unsure, use the nx_docs tool to get relevant, up-to-date docs. Always use this instead of assuming things about nx configuration
35-35: Improve formal wordingPrefer concise, neutral phrasing.
- - Use the terminal output from 'nx_current_running_task_output' to see what's wrong and help the user fix their problem. Use the appropriate tools if necessary + - Use the terminal output from 'nx_current_running_task_output' to diagnose issues and help the user resolve them. Use the appropriate tools if necessary
7-7: Brand capitalizationCapitalize “Nx”.
-You are in an nx workspace using Nx 21.3.11 and pnpm as the package manager. +You are in an Nx workspace using Nx 21.3.11 and pnpm as the package manager.core-java/src/main/java/io/github/boykaframework/builders/Locator.java (2)
52-52: Add brief Javadoc for the new Windows locator fieldKeep platform fields consistently documented.
- private By windows; + /** + * Windows locator. + */ + private By windows;
59-66: Fail fast when Windows locator is missingReturning null here cascades into hard-to-trace NPEs. Throw a clear error instead.
- case WINDOWS -> this.windows; + case WINDOWS -> java.util.Objects.requireNonNull( + this.windows, "Windows locator is not defined for: " + this.name);Confirm whether a fallback to web or mac is desired; if so, we can implement that instead of throwing.
core-java/src/main/java/io/github/boykaframework/enums/OS.java (1)
19-24: Update enum Javadoc: it’s no longer only “Mobile OS”WINDOWS and MAC make this a general OS enum.
-/** - * Mobile OS +/** + * Operating system.core-java/src/main/java/io/github/boykaframework/enums/PlatformType.java (1)
39-41: Javadoc typo: “Map application.” → “Mac application.”Minor spelling fix to avoid confusion.
Example:
/** * Mac application. */core-java/src/main/java/io/github/boykaframework/enums/AutomationType.java (1)
27-31: Doc scope now includes desktopClass Javadoc says “Mobile” but enum now includes Mac/Windows. Consider updating to “Mobile and Desktop.”
core-java/src/main/java/io/github/boykaframework/manager/WindowsManager.java (3)
44-51: Add basic logging/trace (optional)Consider trace logs for parity with other managers.
76-82: Set systemPort only when explicitly providedAvoid sending 0/invalid port.
Apply:
- options.setExperimentalWebDriver (this.settings.isExperimental ()); - options.setSystemPort (this.settings.getSystemPort ()); + options.setExperimentalWebDriver (this.settings.isExperimental ()); + final var port = this.settings.getSystemPort (); + if (port > 0) { + options.setSystemPort (port); + }
84-87: Guard timeouts to skip zero valuesOnly set when > 0 to keep driver defaults.
Apply:
- setOptionIfPresent (ofSeconds (this.settings.getServerStartupTimeout ()), options::setCreateSessionTimeout); - setOptionIfPresent (ofSeconds (this.settings.getCommandTimeout ()), options::setNewCommandTimeout); + final var startup = this.settings.getServerStartupTimeout (); + if (startup > 0) { + options.setCreateSessionTimeout (ofSeconds (startup)); + } + final var command = this.settings.getCommandTimeout (); + if (command > 0) { + options.setNewCommandTimeout (ofSeconds (command)); + }.github/workflows/test-core.yml (1)
272-276: Artifact download: add path and tolerate missing artifactsAlign with other steps and avoid failures when job is skipped.
Apply:
- name: Download reports-testng-desktop-windows files uses: actions/download-artifact@v4 with: name: reports-testng-desktop-windows + path: ${{ github.workspace }}/core-java/reports + # Optional: avoid hard failure if job was skipped + if-no-files-found: ignore
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (15)
.github/instructions/nx.instructions.md(1 hunks).github/workflows/test-core.yml(3 hunks).vscode/settings.json(1 hunks)core-java/src/main/java/io/github/boykaframework/builders/Locator.java(2 hunks)core-java/src/main/java/io/github/boykaframework/config/ui/desktop/machine/ApplicationSetting.java(1 hunks)core-java/src/main/java/io/github/boykaframework/config/ui/desktop/machine/MachineSetting.java(1 hunks)core-java/src/main/java/io/github/boykaframework/enums/AutomationType.java(2 hunks)core-java/src/main/java/io/github/boykaframework/enums/OS.java(1 hunks)core-java/src/main/java/io/github/boykaframework/enums/PlatformType.java(1 hunks)core-java/src/main/java/io/github/boykaframework/manager/DriverManager.java(2 hunks)core-java/src/main/java/io/github/boykaframework/manager/DriverSession.java(1 hunks)core-java/src/main/java/io/github/boykaframework/manager/WindowsManager.java(1 hunks)core-java/test-suites/testng-desktop-windows.xml(1 hunks)package.json(2 hunks)website/package.json(1 hunks)
🧰 Additional context used
🪛 LanguageTool
.github/instructions/nx.instructions.md
[grammar] ~11-~11: There might be a mistake here.
Context: ...est help the user: # General Guidelines - When answering questions, use the nx_wor...
(QB_NEW_EN)
[grammar] ~17-~17: There might be a mistake here.
Context: ...ize_graph' tool # Generation Guidelines If the user wants to generate something,...
(QB_NEW_EN)
[grammar] ~22-~22: There might be a mistake here.
Context: ...user could install a plugin to help them - get generator details using the 'nx_gene...
(QB_NEW_EN)
[grammar] ~31-~31: There might be a mistake here.
Context: ...y were doing # Running Tasks Guidelines If the user wants help with tasks or com...
(QB_NEW_EN)
[grammar] ~32-~32: There might be a mistake here.
Context: ...imilar actions), use the following flow: - Use the 'nx_current_running_tasks_detail...
(QB_NEW_EN)
[grammar] ~33-~33: There might be a mistake here.
Context: ...that were completed, stopped or failed). - If there are any tasks, ask the user if ...
(QB_NEW_EN)
[style] ~35-~35: Consider using a different verb for a more formal wording.
Context: ...' to see what's wrong and help the user fix their problem. Use the appropriate tool...
(FIX_RESOLVE)
🔇 Additional comments (8)
package.json (2)
35-68: Toolchain bumps look coherent (ESLint 9 + TS-ES 8.x + TS 5.9).No red flags; versions are mutually compatible.
103-103: pnpm manager version updated to 10.14.0 — verify workspace-wide consistency.Website subpackage still declares [email protected]. Please align (see website/package.json comment) to avoid install divergence.
.github/instructions/nx.instructions.md (1)
1-41: Auto-generated Nx instructions: content OKDoc is aligned with Nx 21.3.11 usage. Minor wordsmithing nits below.
core-java/src/main/java/io/github/boykaframework/enums/PlatformType.java (1)
45-49: Add WINDOWS platform: LGTMEnum addition looks correct and consistent with existing naming/docs.
core-java/src/main/java/io/github/boykaframework/enums/AutomationType.java (1)
23-23: AutomationName.WINDOWS is supported in appium-java-client v9.5.0, so no fallback is required.core-java/src/main/java/io/github/boykaframework/manager/DriverManager.java (1)
56-59: Start Appium server for WINDOWS: LGTMDesktop server bootstrap extended to Windows correctly.
core-java/src/main/java/io/github/boykaframework/manager/WindowsManager.java (1)
38-41: Settings wiring: LGTMReads desktop machine settings once; good for clarity.
.github/workflows/test-core.yml (1)
225-226: Include Windows in report dependencies: LGTMEnsures Windows results are aggregated.
| test-desktop-windows: | ||
| needs: | ||
| - check-cloud | ||
| uses: ./.github/workflows/_test-core-java.yml | ||
| with: | ||
| runs-on: windows-latest | ||
| suite-name: testng-desktop-windows | ||
| run-appium: true | ||
| run-ios: false | ||
| run-android: false | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Windows test job: verify prerequisites in reusable workflow
Confirm _test-core-java.yml installs and starts Appium with the Windows driver on windows-latest (and that the AUT exists on the runner).
Run:
Expected: reusable workflow installs Appium, Windows driver plugin, starts server; suite file present; AUT (e.g., calc.exe or UWP package) configured.
🏁 Script executed:
#!/bin/bash
fd _test-core-java.yml .github/workflows
rg -nC3 'appium|windows|WinAppDriver|appium-windows-driver' .github/workflows/_test-core-java.yml
rg -nC2 'suite-name:\s*testng-desktop-windows' core-java/test-suitesLength of output: 2319
🏁 Script executed:
#!/bin/bash
echo "Listing all files under core-java/test-suites:"
find core-java/test-suites -maxdepth 1 -type f
echo
echo "Searching for any filename or content containing 'desktop-windows':"
find core-java/test-suites -type f | rg -i 'desktop-windows'Length of output: 1035
Install and start Windows Appium driver in reusable workflow
In .github/workflows/_test-core-java.yml, you install Appium and Android/iOS/macOS drivers but omit the Windows driver (e.g. WinAppDriver or appium-windows-driver) and don’t include it in the appium server --use-drivers flag when runs-on: windows-latest. Add the Windows driver install step and update the server startup command accordingly. Also ensure the AUT referenced in core-java/test-suites/testng-desktop-windows.xml is available on the Windows runner.
🤖 Prompt for AI Agents
In .github/workflows/test-core.yml around lines 168 to 178, the reusable Windows
job launches Appium but the reusable workflow (_test-core-java.yml) doesn't
install or enable the Windows Appium driver; add a step in
.github/workflows/_test-core-java.yml to install the Windows driver (e.g.,
appium-windows-driver or WinAppDriver) when runs-on is windows-latest, and
update the Appium server startup arguments to include the windows driver in the
--use-drivers list; also verify the AUT referenced by
core-java/test-suites/testng-desktop-windows.xml is present on the Windows
runner (or add a setup step to install/deploy it) so tests can run.
| private OS os = MAC; | ||
| private int serverStartupTimeout = 60; | ||
| private boolean showServerLogs; | ||
| private int systemPort = 4724; |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Document and validate systemPort; avoid conflicts with Appium server port.
4724 default is plausible but add constraints and docs.
- private int systemPort = 4724;
+ /**
+ * Dedicated driver/system port to avoid clashing with the Appium server port (commonly 4723).
+ * Valid range: 1024–65535.
+ */
+ private int systemPort = 4724;If validation is centralized in WindowsManager, ensure it rejects out-of-range ports and not-equal to server port.
🏁 Script executed:
#!/bin/bash
# Search for occurrences of systemPort in Java code
echo "Searching for systemPort references..."
rg -n "systemPort" -g "*.java" -C2 || echo "No references found"
# Locate WindowsManager.java if it exists
echo "Locating WindowsManager.java..."
WM_PATH=$(fd --glob "WindowsManager.java" -t f || true)
if [ -n "$WM_PATH" ]; then
echo "Found WindowsManager at: $WM_PATH"
echo "Inspecting for port validation..."
rg -n "port" -C2 "$WM_PATH"
else
echo "WindowsManager.java not found"
fiLength of output: 3664
🏁 Script executed:
#!/bin/bash
WM_FILE=core-java/src/main/java/io/github/boykaframework/manager/WindowsManager.java
echo "---- Class Declaration ----"
rg -n "class WindowsManager" -C3 "$WM_FILE"
echo "---- MachineSetting Usage ----"
rg -n "MachineSetting" -C3 "$WM_FILE"
echo "---- systemPort Reference ----"
rg -n "systemPort" -C3 "$WM_FILE"Length of output: 1356
🏁 Script executed:
#!/bin/bash
# Show snippet around systemPort in MachineSetting.java
FILE=$(fd --glob MachineSetting.java -t f | grep core-java/src/main/java/io/github/boykaframework/config/ui/desktop/machine/MachineSetting.java)
if [ -n "$FILE" ]; then
echo "File: $FILE"
rg -n -C3 "systemPort" "$FILE"
else
echo "MachineSetting.java not found"
fiLength of output: 1099
🏁 Script executed:
#!/bin/bash
# Display Javadoc usage pattern in MachineSetting.java
FILE="core-java/src/main/java/io/github/boykaframework/config/ui/desktop/machine/MachineSetting.java"
if [ -f "$FILE" ]; then
echo "---- Class and field Javadoc samples in MachineSetting.java ----"
# Show class Javadoc and first few fields
sed -n '1,50p' "$FILE"
else
echo "$FILE not found"
fiLength of output: 2425
Add Javadoc and enforce valid systemPort
- In MachineSetting.java, annotate
systemPortwith Javadoc specifying default 4724, valid range 1024–65535, and note to avoid Appium server’s default port 4723. - Implement validation (in the setter or WindowsManager) to reject values outside 1024–65535 or equal to 4723.
- private int systemPort = 4724;
+ /**
+ * Dedicated driver/system port (default: 4724).
+ * Valid range: 1024–65535; must not equal Appium server port 4723.
+ */
+ private int systemPort = 4724;Apply corresponding checks in WindowsManager (core-java/.../manager/WindowsManager.java).
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
core-java/src/main/java/io/github/boykaframework/config/ui/desktop/machine/MachineSetting.java
around line 46, add a Javadoc on the systemPort field stating the default is
4724, valid range is 1024–65535, and to avoid Appium’s default port 4723; then
enforce validation by adding a setter (or updating existing setter) that throws
IllegalArgumentException if the port is <1024 or >65535 or equals 4723. In
core-java/src/main/java/io/github/boykaframework/manager/WindowsManager.java,
add corresponding checks before using the systemPort (validate via the
MachineSetting setter or re-validate there) and fail fast with a clear
exception/log message if the port is out of range or equals 4723 so invalid
configuration cannot be used at runtime.
| final IDriverManager driverManager = switch (this.platformType) { | ||
| case WEB -> new WebDriverManager (); | ||
| case ANDROID -> new AndroidManager (); | ||
| case IOS -> new IOSManager (); | ||
| default -> new MacManager (); | ||
| case MAC -> new MacManager (); | ||
| default -> new WindowsManager (); | ||
| }; |
There was a problem hiding this comment.
Defaulting to WindowsManager is unsafe; add explicit WINDOWS case and fail fast otherwise
“default -> WindowsManager()” will unintentionally route unsupported platforms (e.g., API or future types) to Windows.
Apply:
final IDriverManager driverManager = switch (this.platformType) {
case WEB -> new WebDriverManager ();
case ANDROID -> new AndroidManager ();
case IOS -> new IOSManager ();
- case MAC -> new MacManager ();
- default -> new WindowsManager ();
+ case MAC -> new MacManager ();
+ case WINDOWS -> new WindowsManager ();
+ default -> throw new IllegalStateException ("Unsupported platform: " + this.platformType);
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| final IDriverManager driverManager = switch (this.platformType) { | |
| case WEB -> new WebDriverManager (); | |
| case ANDROID -> new AndroidManager (); | |
| case IOS -> new IOSManager (); | |
| default -> new MacManager (); | |
| case MAC -> new MacManager (); | |
| default -> new WindowsManager (); | |
| }; | |
| final IDriverManager driverManager = switch (this.platformType) { | |
| case WEB -> new WebDriverManager (); | |
| case ANDROID -> new AndroidManager (); | |
| case IOS -> new IOSManager (); | |
| case MAC -> new MacManager (); | |
| case WINDOWS -> new WindowsManager (); | |
| default -> throw new IllegalStateException("Unsupported platform: " + this.platformType); | |
| }; |
🤖 Prompt for AI Agents
In core-java/src/main/java/io/github/boykaframework/manager/DriverManager.java
around lines 60 to 66, the switch currently uses a default -> new
WindowsManager() which wrongly maps any unknown or future platform types to
Windows; add an explicit case for WINDOWS -> new WindowsManager(), and replace
the default branch with a fail-fast branch that throws an
IllegalArgumentException (or IllegalStateException) with a clear message
including the unsupported platform value so unknown enums do not silently fall
back to Windows.
| private void createDriver (final WindowsOptions options) { | ||
| try { | ||
| setDriver (new WindowsDriver (getSession ().getServiceManager () | ||
| .getServiceUrl (), options)); | ||
| } catch (final SessionNotCreatedException e) { | ||
| handleAndThrow (SESSION_NOT_STARTED, e); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Broaden driver creation error handling
Session creation can fail with other RuntimeExceptions. Wrap and rethrow consistently.
Apply:
private void createDriver (final WindowsOptions options) {
try {
setDriver (new WindowsDriver (getSession ().getServiceManager ()
.getServiceUrl (), options));
- } catch (final SessionNotCreatedException e) {
+ } catch (final SessionNotCreatedException e) {
handleAndThrow (SESSION_NOT_STARTED, e);
+ } catch (final RuntimeException e) {
+ handleAndThrow (SESSION_NOT_STARTED, e);
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private void createDriver (final WindowsOptions options) { | |
| try { | |
| setDriver (new WindowsDriver (getSession ().getServiceManager () | |
| .getServiceUrl (), options)); | |
| } catch (final SessionNotCreatedException e) { | |
| handleAndThrow (SESSION_NOT_STARTED, e); | |
| } | |
| } | |
| private void createDriver (final WindowsOptions options) { | |
| try { | |
| setDriver (new WindowsDriver (getSession ().getServiceManager () | |
| .getServiceUrl (), options)); | |
| } catch (final SessionNotCreatedException e) { | |
| handleAndThrow (SESSION_NOT_STARTED, e); | |
| } catch (final RuntimeException e) { | |
| handleAndThrow (SESSION_NOT_STARTED, e); | |
| } | |
| } |
🤖 Prompt for AI Agents
In core-java/src/main/java/io/github/boykaframework/manager/WindowsManager.java
around lines 53 to 60, the try block only catches SessionNotCreatedException but
other RuntimeExceptions during driver/session creation should be handled
consistently; add a catch block for RuntimeException (after the specific
SessionNotCreatedException catch) that calls handleAndThrow(SESSION_NOT_STARTED,
e) to wrap and rethrow these runtime failures the same way as
SessionNotCreatedException.
| private void setApplicationOptions (final WindowsOptions options) { | ||
| final var appSetting = this.settings.getApplication (); | ||
| options.setApp (requireNonNullElse (appSetting.getPath (), appSetting.getBundleId ())); | ||
| setOptionIfPresent (appSetting.getWorkingDir (), options::setAppWorkingDir); | ||
| setOptionIfPresent (appSetting.getArguments (), options::setAppArguments); | ||
| setOptionIfPresent (appSetting.getTopLevelWindow (), options::setAppTopLevelWindow); | ||
| options.setWaitForAppLaunch (ofSeconds (appSetting.getLaunchTimeout ())); | ||
| } |
There was a problem hiding this comment.
Validate app identification; avoid passing null to setApp
Ensure either path or bundleId is provided; fail fast with a clear message.
Apply:
private void setApplicationOptions (final WindowsOptions options) {
final var appSetting = this.settings.getApplication ();
- options.setApp (requireNonNullElse (appSetting.getPath (), appSetting.getBundleId ()));
+ final var app = requireNonNullElse (appSetting.getPath (), appSetting.getBundleId ());
+ if (app == null) {
+ throw new IllegalArgumentException ("Windows app not configured: set application.path or application.bundleId");
+ }
+ options.setApp (app);
setOptionIfPresent (appSetting.getWorkingDir (), options::setAppWorkingDir);
setOptionIfPresent (appSetting.getArguments (), options::setAppArguments);
setOptionIfPresent (appSetting.getTopLevelWindow (), options::setAppTopLevelWindow);
options.setWaitForAppLaunch (ofSeconds (appSetting.getLaunchTimeout ()));
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In core-java/src/main/java/io/github/boykaframework/manager/WindowsManager.java
around lines 62 to 69, currently setApp may receive null when both
appSetting.getPath() and appSetting.getBundleId() are absent; change to
explicitly resolve the identifier into a local variable, validate it is not null
and throw an IllegalArgumentException with a clear message (e.g. "Application
identification required: provide path or bundleId") if missing, then call
options.setApp(...) with the validated value; keep the existing
setOptionIfPresent calls and wait-for-launch logic unchanged.
| <suite name="Boyka Framework Desktop MacOS Suite" verbose="2"> | ||
| <listeners> | ||
| <listener class-name="io.github.boykaframework.testng.listeners.TestResultListener"/> | ||
| </listeners> | ||
| <test name="Test Desktop on MacOS machine"> | ||
| <classes> | ||
| <class name="io.github.boykaframework.testng.ui.mac.CalculatorTest"/> | ||
| </classes> | ||
| </test> | ||
| </suite> |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Suite/test names and class reference still point to Mac; switch to Windows
This will execute Mac tests under the Windows suite and mislabel reports.
-<suite name="Boyka Framework Desktop MacOS Suite" verbose="2">
+<suite name="Boyka Framework Desktop Windows Suite" verbose="2">
@@
- <test name="Test Desktop on MacOS machine">
+ <test name="Test Desktop on Windows machine">
<classes>
- <class name="io.github.boykaframework.testng.ui.mac.CalculatorTest"/>
+ <!-- Update to the correct Windows test class -->
+ <class name="io.github.boykaframework.testng.ui.windows.CalculatorTest"/>
</classes>
</test>
</suite>Run this to locate the actual Windows test class to reference:
🏁 Script executed:
#!/bin/bash
fd -a 'CalculatorTest.java' core-java | rg -nP -C2 'package\s+io\.github\.boykaframework\.testng\.ui\.(windows|mac)\b|class\s+CalculatorTest\b'Length of output: 143
🏁 Script executed:
#!/bin/bash
# Locate all CalculatorTest.java files in the repo
find . -type f -name "CalculatorTest.java"
# Display class declarations for CalculatorTest in any .java file
rg -n "class CalculatorTest" -g "*.java"
# Find any Java files declaring a windows package under ui
rg -n "package .*\\.ui\\.windows" -g "*.java"Length of output: 332
Windows suite XML references non-existent Mac test class
core-java/test-suites/testng-desktop-windows.xmlstill points atio.github.boykaframework.testng.ui.mac.CalculatorTest, but noui.windows.CalculatorTestexists undercore-java/src/test/java- Either create the Windows variant of
CalculatorTestin packageio.github.boykaframework.testng.ui.windowsand update the XML, or rename the suite to reflect that it runs the Mac test class
🤖 Prompt for AI Agents
In core-java/test-suites/testng-desktop-windows.xml around lines 18 to 27, the
suite is named and intended for Windows but references the Mac test class
io.github.boykaframework.testng.ui.mac.CalculatorTest; fix by either (A)
creating a Windows test class at
core-java/src/test/java/io/github/boykaframework/testng/ui/windows/CalculatorTest
that mirrors the Mac test API and update the XML class entry to
io.github.boykaframework.testng.ui.windows.CalculatorTest, or (B) if this suite
is meant to run the Mac tests, rename the suite and test names in this XML to
indicate MacOS (e.g., change suite and test name strings) so they match the
referenced class; apply whichever option matches the intended platform and
ensure package/class names and XML entries are consistent.
| "react": "^19.1.1", | ||
| "react-dom": "^19.1.1", |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
React 19 bump: verify Docusaurus and react-typist peer-compatibility.
Risk of peer range mismatches (build-time warnings or runtime issues). Please verify before merging.
Run:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
cd website
npm view @docusaurus/[email protected] peerDependencies
npm view @docusaurus/[email protected] peerDependencies
npm view @docusaurus/[email protected] peerDependencies
npm view [email protected] peerDependencies
pnpm i --lockfile-only && pnpm -s why react && pnpm -s why react-domLength of output: 929
Upgrade or replace react-typist for React 19 compatibility
@docusaurus/core, theme-classic, and plugin-google-tag-manager all support React ^18||^19, but [email protected]’s peerDependencies only allow React ^0.14‒^16.0. Update or replace react-typist to avoid build/runtime failures.
🤖 Prompt for AI Agents
In website/package.json around lines 25-26, [email protected] has
peerDependencies that only allow React ^0.14–^16 and will break with React 19;
replace or upgrade it to a React-19-compatible alternative. Remove
[email protected] from dependencies, add a maintained replacement (for example
a compatible fork or package such as react-simple-typewriter or react-typist-rc)
or reimplement the typing effect with a small custom component, update all
imports/usages in the codebase to the new API, run npm/yarn install to update
the lockfile, and verify the site builds and runs with React 19.
Closes: #519
Type of changes?
Checklist for Java project
Checklist for Website
Reviewers
@BoykaFramework/boyka-core
Important
Make sure to check the
Allow edits from maintainersbox below this windowSummary by CodeRabbit
New Features
Tests
Documentation
Chores