You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains 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
This PR changes the order of the endpoints used to discover the Firefox versions when managing this browser. Now, the endpoint for stable Firefox versions is used first. For example:
selenium-manager.exe --browser firefox --browser-version 127 --debug
[2024-10-17T15:58:44.733Z DEBUG] geckodriver not found in PATH
[2024-10-17T15:58:44.735Z DEBUG] firefox detected at C:\Program Files\Mozilla Firefox\firefox.exe
[2024-10-17T15:58:44.737Z DEBUG] Running command: wmic datafile where name='C:\\Program Files\\Mozilla Firefox\\firefox.exe' get Version /value
[2024-10-17T15:58:44.818Z DEBUG] Output: "\r\r\n\r\r\nVersion=131.0.3.223\r\r\n\r\r\n\r\r\n\r"
[2024-10-17T15:58:44.823Z DEBUG] Detected browser: firefox 131.0.3.223
[2024-10-17T15:58:44.823Z DEBUG] Discovered firefox version (131) different to specified browser version (127)
[2024-10-17T15:58:45.197Z DEBUG] Required browser: firefox 127.0.2
[2024-10-17T15:58:45.198Z DEBUG] Downloading firefox 127.0.2 from https://ftp.mozilla.org/pub/firefox/releases/127.0.2/win64/en-US/Firefox%20Setup%20127.0.2.exe
[2024-10-17T15:59:16.638Z DEBUG] firefox 127.0.2 is available at C:\Users\boni\.cache\selenium\firefox\win64\127.0.2\firefox.exe
[2024-10-17T15:59:16.674Z DEBUG] Valid geckodriver versions for firefox 127: ["0.35.0", "0.34.0"]
[2024-10-17T15:59:16.675Z DEBUG] Required driver: geckodriver 0.35.0
[2024-10-17T15:59:16.678Z DEBUG] geckodriver 0.35.0 already in the cache
[2024-10-17T15:59:16.678Z INFO ] Driver path: C:\Users\boni\.cache\selenium\geckodriver\win64\0.35.0\geckodriver.exe
[2024-10-17T15:59:16.680Z INFO ] Browser path: C:\Users\boni\.cache\selenium\firefox\win64\127.0.2\firefox.exe
Here are some key observations to aid the review process:
⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review
Logic Change The order of endpoints for requesting Firefox versions has been changed. Verify if this change aligns with the intended behavior and doesn't break any existing functionality.
Refactor endpoint checking logic to use a loop for improved maintainability and extensibility
Consider using a loop or iterator to cycle through the endpoints instead of nested if statements. This can make the code more maintainable and easier to extend if more endpoints are added in the future.
-let mut firefox_versions =- self.request_versions_from_online(FIREFOX_HISTORY_ENDPOINT)?;+let endpoints = [+ FIREFOX_HISTORY_ENDPOINT,+ FIREFOX_HISTORY_MAJOR_ENDPOINT,+ FIREFOX_HISTORY_DEV_ENDPOINT,+];+let mut firefox_versions = Vec::new();+for endpoint in endpoints.iter() {+ firefox_versions = self.request_versions_from_online(endpoint)?;+ if !firefox_versions.is_empty() {+ break;+ }+}
if firefox_versions.is_empty() {
- firefox_versions =- self.request_versions_from_online(FIREFOX_HISTORY_MAJOR_ENDPOINT)?;- if firefox_versions.is_empty() {- firefox_versions =- self.request_versions_from_online(FIREFOX_HISTORY_DEV_ENDPOINT)?;- if firefox_versions.is_empty() {
Apply this suggestion
Suggestion importance[1-10]: 8
Why: Refactoring the code to use a loop instead of nested if statements enhances maintainability and readability. It also makes the code easier to extend if more endpoints are added in the future, which is a valuable improvement.
8
Enhancement
Add error logging for failed endpoint requests to improve debugging capabilities
Consider adding error logging or handling when each endpoint request fails. This can help in troubleshooting if all endpoints fail to return versions.
let mut firefox_versions =
- self.request_versions_from_online(FIREFOX_HISTORY_ENDPOINT)?;+ self.request_versions_from_online(FIREFOX_HISTORY_ENDPOINT)+ .map_err(|e| log::warn!("Failed to fetch from FIREFOX_HISTORY_ENDPOINT: {}", e))?;
if firefox_versions.is_empty() {
firefox_versions =
- self.request_versions_from_online(FIREFOX_HISTORY_MAJOR_ENDPOINT)?;+ self.request_versions_from_online(FIREFOX_HISTORY_MAJOR_ENDPOINT)+ .map_err(|e| log::warn!("Failed to fetch from FIREFOX_HISTORY_MAJOR_ENDPOINT: {}", e))?;
if firefox_versions.is_empty() {
firefox_versions =
- self.request_versions_from_online(FIREFOX_HISTORY_DEV_ENDPOINT)?;+ self.request_versions_from_online(FIREFOX_HISTORY_DEV_ENDPOINT)+ .map_err(|e| log::warn!("Failed to fetch from FIREFOX_HISTORY_DEV_ENDPOINT: {}", e))?;
if firefox_versions.is_empty() {
Apply this suggestion
Suggestion importance[1-10]: 7
Why: Adding error logging for failed endpoint requests can significantly aid in debugging and troubleshooting, making it easier to identify which endpoint failed. This enhancement is relevant and can improve the robustness of the code.
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.
User description
Description
This PR changes the order of the endpoints used to discover the Firefox versions when managing this browser. Now, the endpoint for stable Firefox versions is used first. For example:
Motivation and Context
Fix for #14536.
Types of changes
Checklist
PR Type
enhancement
Description
Changes walkthrough 📝
firefox.rs
Prioritize stable version endpoint for Firefox version requests
rust/src/firefox.rs
endpoint.