[CUS-10952] ADDED NLP FOR MOBILE WEB APPLICATION.#348
Open
ManojTestsigma wants to merge 1 commit intodevfrom
Open
[CUS-10952] ADDED NLP FOR MOBILE WEB APPLICATION.#348ManojTestsigma wants to merge 1 commit intodevfrom
ManojTestsigma wants to merge 1 commit intodevfrom
Conversation
📝 WalkthroughWalkthroughA new MockGeoLocationAction class is introduced for mobile web testing, enabling geolocation override functionality. The implementation validates latitude, longitude, and accuracy inputs, then applies them via Chrome DevTools with cascading fallbacks to CDP mechanisms if initial attempts fail. Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Test Framework
participant Action as MockGeoLocationAction
participant WebDriver as WebDriver/ChromiumDriver
participant DevTools as Chrome DevTools
participant CDP as CDP Command Handler
Test->>Action: execute() with lat, long, accuracy
Action->>Action: Validate inputs (numeric, accuracy ≥ 0)
alt Validation Success
Action->>Action: Build geolocation payload
Action->>DevTools: Attempt DevTools session
alt DevTools Available
Action->>DevTools: Send Emulation.setGeolocationOverride
DevTools-->>Action: Command executed
Action-->>Test: RESULT.SUCCESS
else DevTools Unavailable
Action->>WebDriver: Check for CDP support (HasCdp/ChromiumDriver)
alt HasCdp Available
WebDriver->>CDP: executeCdpCommand(Emulation.setGeolocationOverride)
CDP-->>WebDriver: Command executed
WebDriver-->>Action: Success
Action-->>Test: RESULT.SUCCESS
else CDP via Reflection
Action->>WebDriver: executeCdpCommand via RemoteWebDriver reflection
WebDriver->>CDP: Execute command
CDP-->>WebDriver: Command executed
WebDriver-->>Action: Success
Action-->>Test: RESULT.SUCCESS
end
end
else Validation Failed
Action->>Action: Log error details
Action-->>Test: RESULT.FAILED with error message
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@mock_geo_location/src/main/java/com/testsigma/addons/mobileweb/MockGeoLocationAction.java`:
- Around line 105-116: The reflection-based RemoteWebDriver fallback in
MockGeoLocationAction (the block using
RemoteWebDriver.class.getDeclaredMethod("execute", ...) and invoking
"executeCdpCommand") is non-functional for CDP; remove this reflection hack and
replace it with clear error handling: detect lack of CDP support (check for
instances of HasCdp or ChromiumDriver earlier) and when neither is available
throw or log a descriptive UnsupportedOperationException/RuntimeException that
includes the driver class name and a message that CDP is not supported, ensuring
no silent fallback is attempted.
- Around line 49-55: The latitude/longitude parsing currently only checks
numeric format in MockGeoLocationAction (see parsing of latVal and longVal) but
misses range validation; after parsing latitude in the block that sets
"latitude" and before continuing, add a check that latitude is between -90 and
90 and, if out of range, call setErrorMessage with a clear message and return
Result.FAILED; do the same for longitude (range -180 to 180) in the block where
"longitude" is parsed, using the same pattern (setErrorMessage + return
Result.FAILED) and keep existing logging via logger.info for valid values.
Comment on lines
+105
to
+116
| } else if (this.driver instanceof RemoteWebDriver) { | ||
| // Reflection hack for RemoteWebDriver to bypass HasDevTools/Augmenter | ||
| Method execute = RemoteWebDriver.class.getDeclaredMethod("execute", String.class, Map.class); | ||
| execute.setAccessible(true); | ||
| Map<String, Object> params = new HashMap<>(); | ||
| params.put("cmd", "Emulation.setGeolocationOverride"); | ||
| params.put("params", coordinates); | ||
| execute.invoke(this.driver, "executeCdpCommand", params); | ||
| logger.info("Fallback to RemoteWebDriver execution successful."); | ||
| } else { | ||
| throw e; // Rethrow if no fallback is possible | ||
| } |
There was a problem hiding this comment.
Reflection-based fallback is likely non-functional.
The RemoteWebDriver.execute() method is an internal method for W3C WebDriver protocol commands, not CDP commands. Passing "executeCdpCommand" as the command name won't work because it's not a recognized W3C command. The nested params structure also doesn't match the expected format.
If the driver doesn't implement HasCdp or isn't a ChromiumDriver, it likely doesn't support CDP at all. Consider removing this fallback or providing a clear error message indicating CDP is not supported.
Proposed fix to replace with clear error handling
} else if (this.driver instanceof ChromiumDriver) {
((ChromiumDriver) this.driver).executeCdpCommand("Emulation.setGeolocationOverride", coordinates);
logger.info("Fallback to ChromiumDriver (Chrome/Edge) execution successful.");
- } else if (this.driver instanceof RemoteWebDriver) {
- // Reflection hack for RemoteWebDriver to bypass HasDevTools/Augmenter
- Method execute = RemoteWebDriver.class.getDeclaredMethod("execute", String.class, Map.class);
- execute.setAccessible(true);
- Map<String, Object> params = new HashMap<>();
- params.put("cmd", "Emulation.setGeolocationOverride");
- params.put("params", coordinates);
- execute.invoke(this.driver, "executeCdpCommand", params);
- logger.info("Fallback to RemoteWebDriver execution successful.");
} else {
- throw e; // Rethrow if no fallback is possible
+ throw new UnsupportedOperationException(
+ "Driver does not support CDP commands. Geolocation override requires Chrome/Chromium-based browser with DevTools support.");
}📝 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.
Suggested change
| } else if (this.driver instanceof RemoteWebDriver) { | |
| // Reflection hack for RemoteWebDriver to bypass HasDevTools/Augmenter | |
| Method execute = RemoteWebDriver.class.getDeclaredMethod("execute", String.class, Map.class); | |
| execute.setAccessible(true); | |
| Map<String, Object> params = new HashMap<>(); | |
| params.put("cmd", "Emulation.setGeolocationOverride"); | |
| params.put("params", coordinates); | |
| execute.invoke(this.driver, "executeCdpCommand", params); | |
| logger.info("Fallback to RemoteWebDriver execution successful."); | |
| } else { | |
| throw e; // Rethrow if no fallback is possible | |
| } | |
| } else if (this.driver instanceof ChromiumDriver) { | |
| ((ChromiumDriver) this.driver).executeCdpCommand("Emulation.setGeolocationOverride", coordinates); | |
| logger.info("Fallback to ChromiumDriver (Chrome/Edge) execution successful."); | |
| } else { | |
| throw new UnsupportedOperationException( | |
| "Driver does not support CDP commands. Geolocation override requires Chrome/Chromium-based browser with DevTools support."); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@mock_geo_location/src/main/java/com/testsigma/addons/mobileweb/MockGeoLocationAction.java`
around lines 105 - 116, The reflection-based RemoteWebDriver fallback in
MockGeoLocationAction (the block using
RemoteWebDriver.class.getDeclaredMethod("execute", ...) and invoking
"executeCdpCommand") is non-functional for CDP; remove this reflection hack and
replace it with clear error handling: detect lack of CDP support (check for
instances of HasCdp or ChromiumDriver earlier) and when neither is available
throw or log a descriptive UnsupportedOperationException/RuntimeException that
includes the driver class name and a message that CDP is not supported, ensuring
no silent fallback is attempted.
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
1 participant
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.
please review this addon and publish as PUBLIC
Addon name : MOCK_GEO_LOCATION
Addon accont: https://jarvis.testsigma.com/ui/tenants/2817/addons
Jira: https://testsigma.atlassian.net/browse/CUS-10952
fix
Summary by CodeRabbit