Open selenium-ci opened 1 week ago
⏱️ Estimated effort to review [1-5] | 2 |
🧪 Relevant tests | No |
🔒 Security concerns | No |
⚡ Key issues to review | None |
Category | Suggestion | Score |
Maintainability |
Use a variable for the version number to avoid hardcoding it multiple times___ **Consider using a variable for the version number to avoid hardcoding it multiple times,which will make future updates easier and reduce the risk of errors.** [common/repositories.bzl [14]](https://github.com/SeleniumHQ/selenium/pull/14158/files#diff-25d82cd18102fed27d3202000e1f1b3a56a85ad2848236d91989cd30a3952401R14-R14) ```diff -url = "https://ftp.mozilla.org/pub/firefox/releases/127.0.1/linux-x86_64/en-US/firefox-127.0.1.tar.bz2", +version = "127.0.1" +url = f"https://ftp.mozilla.org/pub/firefox/releases/{version}/linux-x86_64/en-US/firefox-{version}.tar.bz2", ``` - [ ] **Apply this suggestion** Suggestion importance[1-10]: 7Why: Using a variable for the version number is a good practice for maintainability and reducing errors during updates. This suggestion directly addresses a repeated pattern in the PR. | 7 |
Enhancement |
Add a retry mechanism for downloading the archives to handle transient network issues___ **Consider adding a retry mechanism for downloading the archives to handle transient networkissues more gracefully.** [common/repositories.bzl [14]](https://github.com/SeleniumHQ/selenium/pull/14158/files#diff-25d82cd18102fed27d3202000e1f1b3a56a85ad2848236d91989cd30a3952401R14-R14) ```diff url = "https://ftp.mozilla.org/pub/firefox/releases/127.0.1/linux-x86_64/en-US/firefox-127.0.1.tar.bz2", +# Add retry mechanism for downloading the archive ``` - [ ] **Apply this suggestion** Suggestion importance[1-10]: 6Why: Adding a retry mechanism is a practical enhancement for handling network instability. This suggestion is relevant given the context of downloading files from external sources. | 6 |
Security |
Add a comment to verify the SHA256 hash after downloading the file to ensure integrity___ **Consider adding a comment or a mechanism to verify the integrity of the downloaded files,such as checking the SHA256 hash after download, to ensure that the files have not been tampered with.** [common/repositories.bzl [14-15]](https://github.com/SeleniumHQ/selenium/pull/14158/files#diff-25d82cd18102fed27d3202000e1f1b3a56a85ad2848236d91989cd30a3952401R14-R15) ```diff url = "https://ftp.mozilla.org/pub/firefox/releases/127.0.1/linux-x86_64/en-US/firefox-127.0.1.tar.bz2", -sha256 = "57564e6219f8f79418ba49d1e7a6edf44f8e253f777d0ae7de7dbff200c3d5f4", +sha256 = "57564e6219f8f79418ba49d1e7a6edf44f8e253f777d0ae7de7dbff200c3d5f4", # Ensure to verify this SHA256 hash after download ``` - [ ] **Apply this suggestion** Suggestion importance[1-10]: 5Why: The suggestion is valid as it enhances security by verifying the integrity of downloads. However, the existing code already includes SHA256 hashes which serve this purpose, so the added comment might be somewhat redundant. | 5 |
Best practice |
Ensure consistency of URLs and SHA256 hashes across all platforms___ **Ensure that the URLs and SHA256 hashes are updated consistently across all platforms toavoid mismatches and potential issues during the build process.** [common/repositories.bzl [14-15]](https://github.com/SeleniumHQ/selenium/pull/14158/files#diff-25d82cd18102fed27d3202000e1f1b3a56a85ad2848236d91989cd30a3952401R14-R15) ```diff url = "https://ftp.mozilla.org/pub/firefox/releases/127.0.1/linux-x86_64/en-US/firefox-127.0.1.tar.bz2", -sha256 = "57564e6219f8f79418ba49d1e7a6edf44f8e253f777d0ae7de7dbff200c3d5f4", +sha256 = "57564e6219f8f79418ba49d1e7a6edf44f8e253f777d0ae7de7dbff200c3d5f4", # Ensure consistency across all platforms ``` - [ ] **Apply this suggestion** Suggestion importance[1-10]: 5Why: The suggestion emphasizes consistency, which is important. However, the PR already shows consistent updates across different platforms, making this suggestion somewhat unnecessary in this context. | 5 |
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 57.18%. Comparing base (
57f8398
) to head (ed48dc5
). Report is 574 commits behind head on trunk.:exclamation: Current head ed48dc5 differs from pull request most recent head 54429e9
Please upload reports for the commit 54429e9 to get more accurate results.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
**Action:** .NET / Browser Tests / Browser Tests |
**Failed stage:** [Run Bazel](https://github.com/SeleniumHQ/selenium/actions/runs/9720094983/job/26831078335) [❌] |
**Failed test name:** ElementFindingTest-firefox, ElementFindingTest-chrome |
**Failure summary:**
The action failed because two tests, ElementFindingTest-firefox and ElementFindingTest-chrome , failed to execute. The specific reasons for the failures are: ElementFindingTest-firefox : The test process could not be started due to an issue with shortening the executable path. ElementFindingTest-chrome : Similar to the Firefox test, the test process could not be started because the executable path could not be shortened enough. |
Relevant error logs:```yaml 1: ##[group]Operating System 2: Microsoft Windows Server 2022 ... 767: [32m[1,245 / 2,103][0m Compiling src/google/protobuf/util/internal/field_mask_utility.cc [for tool]; 1s local ... (4 actions, 3 running) 768: [32m[1,254 / 2,103][0m Compiling src/google/protobuf/util/internal/field_mask_utility.cc [for tool]; 2s local ... (4 actions, 3 running) 769: [32m[1,258 / 2,103][0m Compiling src/google/protobuf/util/internal/field_mask_utility.cc [for tool]; 3s local ... (4 actions running) 770: [32m[1,262 / 2,103][0m Action java/src/org/openqa/selenium/manager/libmanager-module.jar; 1s local ... (4 actions running) 771: [32m[1,265 / 2,103][0m Action java/src/org/openqa/selenium/manager/libmanager-module.jar; 2s local ... (4 actions running) 772: [32m[1,268 / 2,103][0m Building Python zip: //common/devtools:pdl_to_json [for tool]; 3s local ... (4 actions running) 773: [32m[1,269 / 2,103][0m Building Python zip: //common/devtools:pdl_to_json [for tool]; 6s local ... (4 actions, 3 running) 774: [32mINFO: [0mFrom Building java/src/org/openqa/selenium/remote/libapi-class.jar (66 source files): 775: java\src\org\openqa\selenium\remote\ErrorHandler.java:46: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 776: private final ErrorCodes errorCodes; 777: ^ 778: java\src\org\openqa\selenium\remote\ErrorHandler.java:60: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 779: this.errorCodes = new ErrorCodes(); 780: ^ 781: java\src\org\openqa\selenium\remote\ErrorHandler.java:68: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 782: public ErrorHandler(ErrorCodes codes, boolean includeServerErrors) { 783: ^ 784: java\src\org\openqa\selenium\remote\Response.java:97: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 785: ErrorCodes errorCodes = new ErrorCodes(); 786: ^ 787: java\src\org\openqa\selenium\remote\Response.java:97: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 788: ErrorCodes errorCodes = new ErrorCodes(); 789: ^ 790: java\src\org\openqa\selenium\remote\ProtocolHandshake.java:181: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 791: response.setStatus(ErrorCodes.SUCCESS); 792: ^ 793: java\src\org\openqa\selenium\remote\ProtocolHandshake.java:182: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 794: response.setState(ErrorCodes.SUCCESS_STRING); 795: ^ 796: java\src\org\openqa\selenium\remote\W3CHandshakeResponse.java:53: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 797: new ErrorCodes().toStatus((String) rawError, Optional.of(tuple.getStatusCode()))); 798: ^ 799: java\src\org\openqa\selenium\remote\W3CHandshakeResponse.java:56: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 800: new ErrorCodes().getExceptionType((String) rawError); 801: ^ 802: java\src\org\openqa\selenium\remote\codec\AbstractHttpResponseCodec.java:44: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 803: private final ErrorCodes errorCodes = new ErrorCodes(); 804: ^ 805: java\src\org\openqa\selenium\remote\codec\AbstractHttpResponseCodec.java:44: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 806: private final ErrorCodes errorCodes = new ErrorCodes(); 807: ^ 808: java\src\org\openqa\selenium\remote\codec\AbstractHttpResponseCodec.java:55: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 809: int status = response.getStatus() == ErrorCodes.SUCCESS ? HTTP_OK : HTTP_INTERNAL_ERROR; 810: ^ 811: java\src\org\openqa\selenium\remote\codec\AbstractHttpResponseCodec.java:101: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 812: response.setStatus(ErrorCodes.UNKNOWN_COMMAND); 813: ^ 814: java\src\org\openqa\selenium\remote\codec\AbstractHttpResponseCodec.java:103: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 815: response.setStatus(ErrorCodes.UNHANDLED_ERROR); 816: ^ 817: java\src\org\openqa\selenium\remote\codec\AbstractHttpResponseCodec.java:124: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 818: response.setStatus(ErrorCodes.SUCCESS); 819: ^ 820: java\src\org\openqa\selenium\remote\codec\AbstractHttpResponseCodec.java:125: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 821: response.setState(errorCodes.toState(ErrorCodes.SUCCESS)); 822: ^ 823: java\src\org\openqa\selenium\remote\codec\AbstractHttpResponseCodec.java:131: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 824: response.setState(errorCodes.toState(ErrorCodes.SUCCESS)); 825: ^ 826: java\src\org\openqa\selenium\remote\codec\w3c\W3CHttpResponseCodec.java:70: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 827: private final ErrorCodes errorCodes = new ErrorCodes(); 828: ^ 829: java\src\org\openqa\selenium\remote\codec\w3c\W3CHttpResponseCodec.java:70: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 830: private final ErrorCodes errorCodes = new ErrorCodes(); 831: ^ 832: java\src\org\openqa\selenium\remote\codec\w3c\W3CHttpResponseCodec.java:93: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 833: response.setStatus(ErrorCodes.UNKNOWN_COMMAND); 834: ^ 835: java\src\org\openqa\selenium\remote\codec\w3c\W3CHttpResponseCodec.java:98: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 836: response.setStatus(ErrorCodes.UNHANDLED_ERROR); 837: ^ 838: java\src\org\openqa\selenium\remote\codec\w3c\W3CHttpResponseCodec.java:145: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 839: response.setStatus(ErrorCodes.SUCCESS); ... 920: bazel-out\x64_windows-fastbuild-ST-63230e73852e\bin\dotnet\src\webdriver\cdp\v126\Target\TargetInfo.cs(21,149): warning CS1570: XML comment has badly formed XML -- 'Reference to undefined entity 'q'.' 921: [32m[2,101 / 2,103][0m [Prepa] Compiling ElementFindingTest-chrome ... (2 actions, 0 running) 922: [32m[2,101 / 2,103][0m Compiling ElementFindingTest-chrome; 1s local ... (2 actions running) 923: [32m[2,103 / 2,105][0m [Prepa] Testing //dotnet/test/common:ElementFindingTest-chrome ... (2 actions, 0 running) 924: [32m[2,103 / 2,105][0m [Prepa] Testing //dotnet/test/common:ElementFindingTest-chrome; 5s ... (2 actions, 0 running) 925: [31m[1mFAIL: [0m//dotnet/test/common:ElementFindingTest-firefox (see D:/_bazel/execroot/_main/bazel-out/x64_windows-fastbuild-ST-3d59667fd571/testlogs/dotnet/test/common/ElementFindingTest-firefox/test.log) 926: [32mINFO: [0mFrom Testing //dotnet/test/common:ElementFindingTest-firefox: 927: ==================== Test output for //dotnet/test/common:ElementFindingTest-firefox: 928: ERROR(tools/test/windows/tw.cc:1302) ERROR: src/main/native/windows/process.cc(95): WaitableProcess::Create(D:\_bazel\execroot\_main\bazel-out\x64_windows-fastbuild-ST-3d59667fd571\bin\dotnet\test\common\ElementFindingTest-firefox\net7.0\WebDriver.Common.Tests.dll.bat.runfiles\_main\dotnet\test\common\ElementFindingTest-firefox\net7.0\WebDriver.Common.Tests.dll.bat): ERROR: src/main/native/windows/util.cc(292): AsExecutablePathForCreateProcess(D:\_bazel\execroot\_main\bazel-out\x64_windows-fastbuild-ST-3d59667fd571\bin\dotnet\test\common\ElementFindingTest-firefox\net7.0\WebDriver.Common.Tests.dll.bat.runfiles\_main\dotnet\test\common\ElementFindingTest-firefox\net7.0\WebDriver.Common.Tests.dll.bat): ERROR: src/main/native/windows/util.cc(262): GetShortPathNameW(\\?\D:\_bazel\execroot\_main\bazel-out\x64_windows-fastbuild-ST-3d59667fd571\bin\dotnet\test\common\ElementFindingTest-firefox\net7.0\WebDriver.Common.Tests.dll.bat.runfiles\_main\dotnet\test\common\ElementFindingTest-firefox\net7.0\WebDriver.Common.Tests.dll.bat): cannot shorten the path enough 929: ERROR(tools/test/windows/tw.cc:1479) Failed to start test process (arg: D:\_bazel\execroot\_main\bazel-out\x64_windows-fastbuild-ST-3d59667fd571\bin\dotnet\test\common\ElementFindingTest-firefox\net7.0\WebDriver.Common.Tests.dll.bat.runfiles\_main\dotnet\test\common\ElementFindingTest-firefox\net7.0\WebDriver.Common.Tests.dll.bat) 930: ================================================================================ 931: [31m[1mFAIL: [0m//dotnet/test/common:ElementFindingTest-chrome (see D:/_bazel/execroot/_main/bazel-out/x64_windows-fastbuild-ST-3d59667fd571/testlogs/dotnet/test/common/ElementFindingTest-chrome/test.log) 932: [32mINFO: [0mFrom Testing //dotnet/test/common:ElementFindingTest-chrome: 933: ==================== Test output for //dotnet/test/common:ElementFindingTest-chrome: 934: ERROR(tools/test/windows/tw.cc:1302) ERROR: src/main/native/windows/process.cc(95): WaitableProcess::Create(D:\_bazel\execroot\_main\bazel-out\x64_windows-fastbuild-ST-3d59667fd571\bin\dotnet\test\common\ElementFindingTest-chrome\net7.0\WebDriver.Common.Tests.dll.bat.runfiles\_main\dotnet\test\common\ElementFindingTest-chrome\net7.0\WebDriver.Common.Tests.dll.bat): ERROR: src/main/native/windows/util.cc(292): AsExecutablePathForCreateProcess(D:\_bazel\execroot\_main\bazel-out\x64_windows-fastbuild-ST-3d59667fd571\bin\dotnet\test\common\ElementFindingTest-chrome\net7.0\WebDriver.Common.Tests.dll.bat.runfiles\_main\dotnet\test\common\ElementFindingTest-chrome\net7.0\WebDriver.Common.Tests.dll.bat): ERROR: src/main/native/windows/util.cc(262): GetShortPathNameW(\\?\D:\_bazel\execroot\_main\bazel-out\x64_windows-fastbuild-ST-3d59667fd571\bin\dotnet\test\common\ElementFindingTest-chrome\net7.0\WebDriver.Common.Tests.dll.bat.runfiles\_main\dotnet\test\common\ElementFindingTest-chrome\net7.0\WebDriver.Common.Tests.dll.bat): cannot shorten the path enough 935: ERROR(tools/test/windows/tw.cc:1479) Failed to start test process (arg: D:\_bazel\execroot\_main\bazel-out\x64_windows-fastbuild-ST-3d59667fd571\bin\dotnet\test\common\ElementFindingTest-chrome\net7.0\WebDriver.Common.Tests.dll.bat.runfiles\_main\dotnet\test\common\ElementFindingTest-chrome\net7.0\WebDriver.Common.Tests.dll.bat) 936: ================================================================================ 937: [32mINFO: [0mFound 2 test targets... 938: [32mINFO: [0mElapsed time: 415.926s, Critical Path: 99.75s 939: //dotnet/test/common:ElementFindingTest-chrome [0m[31m[1mFAILED[0m in 0.1s 940: [32mINFO: [0m2105 processes: 815 internal, 488 local, 802 worker. 941: D:/_bazel/execroot/_main/bazel-out/x64_windows-fastbuild-ST-3d59667fd571/testlogs/dotnet/test/common/ElementFindingTest-chrome/test.log 942: [32mINFO: [0mBuild completed, 2 tests FAILED, 2105 total actions 943: //dotnet/test/common:ElementFindingTest-firefox [0m[31m[1mFAILED[0m in 0.1s 944: D:/_bazel/execroot/_main/bazel-out/x64_windows-fastbuild-ST-3d59667fd571/testlogs/dotnet/test/common/ElementFindingTest-firefox/test.log 945: Executed 2 out of 2 tests: [0m[31m[1m2 fail locally[0m. 946: [0m 947: ##[error]Process completed with exit code 1. ``` |
This is an automated pull request to update pinned browsers and drivers
Merge after verify the new browser versions properly passing the tests and no bugs need to be filed