Closed titusfortner closed 1 week ago
β±οΈ Estimated effort to review [1-5] | 3 |
π§ͺ Relevant tests | No |
π Security concerns | No |
β‘ Key issues to review |
Possible Bug: The workflow is triggered on pull request closure but does not check if the branch is specifically for release-preparation. This could lead to unintended releases if a non-release-preparation branch is mistakenly named in a way that matches the regex. |
Efficiency Concern: The workflow might benefit from conditions that prevent it from running unnecessarily, for example, by checking if the PR is related to a release or not before executing. |
Category | Suggestion | Score |
Possible bug |
Correct the version extraction logic to properly capture the version from the branch name___ **Therun command for extracting the version from the branch name does not actually extract the version. You need to use a regex to capture the version part from the branch name.** [.github/workflows/stage-release.yml [22-25]](https://github.com/SeleniumHQ/selenium/pull/14122/files#diff-7beae040710de3a234065211d66bf51c5bbfd1ccd6a8659966716fca0d955aa5R22-R25) ```diff run: | BRANCH_NAME="${{ github.event.pull_request.head.ref }}" - VERSION="${BASH_REMATCH[1]}" - echo "VERSION=$VERSION" >> $GITHUB_ENV + if [[ "$BRANCH_NAME" =~ release-preparation-(.*) ]]; then + VERSION="${BASH_REMATCH[1]}" + echo "VERSION=$VERSION" >> $GITHUB_ENV + else + echo "Failed to extract version from branch name" + exit 1 + fi ``` - [ ] **Apply this suggestion** Suggestion importance[1-10]: 10Why: The original script does not correctly extract the version from the branch name, which is crucial for the workflow's functionality. The suggested improvement correctly implements regex matching to extract the version, which is essential for the release process. | 10 |
Best practice |
Specify the remote repository in the
___
**The | 7 |
Possible issue |
Ensure
___
**The | 6 |
Enhancement |
Use a more descriptive name for the uploaded artifact to avoid confusion___ **Theupload-artifact action should use a more descriptive name for the artifact to avoid confusion.** [.github/workflows/stage-release.yml [44]](https://github.com/SeleniumHQ/selenium/pull/14122/files#diff-7beae040710de3a234065211d66bf51c5bbfd1ccd6a8659966716fca0d955aa5R44-R44) ```diff -name: release-assets +name: selenium-release-assets ``` - [ ] **Apply this suggestion** Suggestion importance[1-10]: 5Why: While the original name is not incorrect, using a more descriptive name like 'selenium-release-assets' helps in better identifying the artifacts, especially when multiple workflows are present. | 5 |
I verified it was able to execute and create the draft release, but the default release notes generation is generated from the previous release, which is nightly. So I want to explore deleting and redeploying nightly around this workflow.
More importantly, I'm getting failures trying to build some of these assets remotely. Might just be a ruby thing, need to investigate. Will need to try this on the next release π
**Action:** Ruby / Remote Tests (edge, windows) / Remote Tests (edge, windows) |
**Failed stage:** [Run Bazel](https://github.com/SeleniumHQ/selenium/actions/runs/9588441376/job/26440705472) [β] |
**Failed test name:** Selenium::WebDriver::Driver one element raises if invalid locator |
**Failure summary:**
The action failed because the test Selenium::WebDriver::Driver one element raises if invalid locator failed. The test was expected to fail due to a pending guard, but no error was raised. ./rb/spec/integration/selenium/webdriver/driver_spec.rb at line 150. {:browser=>:edge, :platform=>:windows, :reason=>"https://bugs.chromium.org/p/chromedriver/issues/detail?id=4743"} . |
Relevant error logs:```yaml 1: ##[group]Operating System 2: Microsoft Windows Server 2022 ... 671: [32m[2,856 / 3,105][0m Extracting npm package @mui/icons-material@5.15.18_-796748879; 2s disk-cache ... (4 actions, 0 running) 672: [32m[2,935 / 3,105][0m Extracting npm package @mui/icons-material@5.15.18_-796748879; 3s disk-cache ... (4 actions, 0 running) 673: [32m[2,941 / 3,105][0m Installing external/rules_ruby~~ruby~bundle/rb/vendor/cache/bundler-2.2.33.gem (@bundle//:bundler-2.2.33); 1s local, disk-cache ... (4 actions, 1 running) 674: [32m[2,942 / 3,105][0m Installing external/rules_ruby~~ruby~bundle/rb/vendor/cache/bundler-2.2.33.gem (@bundle//:bundler-2.2.33); 2s local, disk-cache ... (4 actions, 1 running) 675: [32mINFO: [0mFrom PackageZip javascript/grid-ui/react-zip.jar: 676: C:\Users\RUNNER~1\AppData\Local\Temp\Bazel.runfiles_hei0s2b5\runfiles\rules_python~~python~python_3_8_x86_64-pc-windows-msvc\lib\zipfile.py:1525: UserWarning: Duplicate name: 'grid-ui/' 677: return self._open_to_write(zinfo, force_zip64=force_zip64) 678: [32mINFO: [0mFrom Building java/src/org/openqa/selenium/remote/libapi-class.jar (66 source files): 679: java\src\org\openqa\selenium\remote\ErrorHandler.java:46: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 680: private final ErrorCodes errorCodes; 681: ^ 682: java\src\org\openqa\selenium\remote\ErrorHandler.java:60: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 683: this.errorCodes = new ErrorCodes(); 684: ^ 685: java\src\org\openqa\selenium\remote\ErrorHandler.java:68: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 686: public ErrorHandler(ErrorCodes codes, boolean includeServerErrors) { 687: ^ 688: java\src\org\openqa\selenium\remote\Response.java:97: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 689: ErrorCodes errorCodes = new ErrorCodes(); 690: ^ 691: java\src\org\openqa\selenium\remote\Response.java:97: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 692: ErrorCodes errorCodes = new ErrorCodes(); 693: ^ 694: java\src\org\openqa\selenium\remote\ProtocolHandshake.java:181: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 695: response.setStatus(ErrorCodes.SUCCESS); 696: ^ 697: java\src\org\openqa\selenium\remote\ProtocolHandshake.java:182: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 698: response.setState(ErrorCodes.SUCCESS_STRING); 699: ^ 700: java\src\org\openqa\selenium\remote\W3CHandshakeResponse.java:53: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 701: new ErrorCodes().toStatus((String) rawError, Optional.of(tuple.getStatusCode()))); 702: ^ 703: java\src\org\openqa\selenium\remote\W3CHandshakeResponse.java:56: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 704: new ErrorCodes().getExceptionType((String) rawError); 705: ^ 706: 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 707: private final ErrorCodes errorCodes = new ErrorCodes(); 708: ^ 709: 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 710: private final ErrorCodes errorCodes = new ErrorCodes(); 711: ^ 712: 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 713: int status = response.getStatus() == ErrorCodes.SUCCESS ? HTTP_OK : HTTP_INTERNAL_ERROR; 714: ^ 715: 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 716: response.setStatus(ErrorCodes.UNKNOWN_COMMAND); 717: ^ 718: 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 719: response.setStatus(ErrorCodes.UNHANDLED_ERROR); 720: ^ 721: 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 722: response.setStatus(ErrorCodes.SUCCESS); 723: ^ 724: 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 725: response.setState(errorCodes.toState(ErrorCodes.SUCCESS)); 726: ^ 727: 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 728: response.setState(errorCodes.toState(ErrorCodes.SUCCESS)); 729: ^ 730: 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 731: private final ErrorCodes errorCodes = new ErrorCodes(); 732: ^ 733: 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 734: private final ErrorCodes errorCodes = new ErrorCodes(); 735: ^ 736: 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 737: response.setStatus(ErrorCodes.UNKNOWN_COMMAND); 738: ^ 739: 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 740: response.setStatus(ErrorCodes.UNHANDLED_ERROR); 741: ^ 742: 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 743: response.setStatus(ErrorCodes.SUCCESS); ... 891: [32m[3,113 / 3,128][0m 8 / 28 tests;[0m [Sched] Testing //rb/spec/integration/selenium/webdriver/edge:driver-edge-remote; 40s ... (4 actions, 2 running) 892: [32m[3,114 / 3,128][0m 9 / 28 tests;[0m [Sched] Testing //rb/spec/integration/selenium/webdriver/edge:driver-edge-remote; 41s ... (4 actions, 1 running) 893: [32m[3,114 / 3,128][0m 9 / 28 tests;[0m [Sched] Testing //rb/spec/integration/selenium/webdriver/edge:driver-edge-remote; 51s ... (4 actions, 1 running) 894: [32m[3,114 / 3,128][0m 9 / 28 tests;[0m [Sched] Testing //rb/spec/integration/selenium/webdriver/edge:driver-edge-remote; 52s ... (4 actions, 1 running) 895: [32m[3,114 / 3,128][0m 9 / 28 tests;[0m [Sched] Testing //rb/spec/integration/selenium/webdriver:guard-edge-remote; 27s ... (4 actions, 2 running) 896: [32m[3,115 / 3,128][0m 10 / 28 tests;[0m [Sched] Testing //rb/spec/integration/selenium/webdriver:guard-edge-remote; 28s ... (4 actions, 1 running) 897: [32m[3,115 / 3,128][0m 10 / 28 tests;[0m [Sched] Testing //rb/spec/integration/selenium/webdriver:guard-edge-remote; 38s ... (4 actions, 1 running) 898: [32m[3,115 / 3,128][0m 10 / 28 tests;[0m [Sched] Testing //rb/spec/integration/selenium/webdriver:guard-edge-remote; 39s ... (4 actions, 1 running) 899: [32m[3,115 / 3,128][0m 10 / 28 tests;[0m [Sched] Testing //rb/spec/integration/selenium/webdriver:error-edge-remote; 39s ... (4 actions, 2 running) 900: [32m[3,116 / 3,128][0m 11 / 28 tests;[0m [Sched] Testing //rb/spec/integration/selenium/webdriver:error-edge-remote; 40s ... (4 actions, 1 running) 901: [32m[3,116 / 3,128][0m 11 / 28 tests;[0m [Sched] Testing //rb/spec/integration/selenium/webdriver:error-edge-remote; 50s ... (4 actions, 1 running) 902: [32m[3,116 / 3,128][0m 11 / 28 tests;[0m [Sched] Testing //rb/spec/integration/selenium/webdriver:error-edge-remote; 51s ... (4 actions, 1 running) ... 940: [32m[3,124 / 3,128][0m 19 / 28 tests;[0m Testing //rb/spec/integration/selenium/webdriver:driver-edge-remote; 164s local, disk-cache ... (4 actions, 2 running) 941: [32m[3,124 / 3,128][0m 19 / 28 tests;[0m Testing //rb/spec/integration/selenium/webdriver:driver-edge-remote; 175s local, disk-cache ... (4 actions, 2 running) 942: [32m[3,124 / 3,128][0m 19 / 28 tests;[0m Testing //rb/spec/integration/selenium/webdriver:driver-edge-remote; 176s local, disk-cache ... (4 actions, 2 running) 943: [32m[3,124 / 3,128][0m 19 / 28 tests;[0m Testing //rb/spec/integration/selenium/webdriver:driver-edge-remote; 189s local, disk-cache ... (4 actions, 2 running) 944: [32m[3,125 / 3,128][0m 20 / 28 tests;[0m Testing //rb/spec/integration/selenium/webdriver:driver-edge-remote; 191s local, disk-cache ... (3 actions, 1 running) 945: [32m[3,125 / 3,128][0m 20 / 28 tests;[0m Testing //rb/spec/integration/selenium/webdriver:driver-edge-remote; 201s local, disk-cache ... (3 actions, 1 running) 946: [32m[3,125 / 3,128][0m 20 / 28 tests;[0m Testing //rb/spec/integration/selenium/webdriver:driver-edge-remote; 226s local, disk-cache ... (3 actions, 2 running) 947: [31m[1mFAIL: [0m//rb/spec/integration/selenium/webdriver:driver-edge-remote (see D:/_bazel/execroot/_main/bazel-out/x64_windows-fastbuild/testlogs/rb/spec/integration/selenium/webdriver/driver-edge-remote/test.log) 948: [31m[1mFAILED: [0m//rb/spec/integration/selenium/webdriver:driver-edge-remote (Summary) ... 976: finds by xpath 977: finds by css selector 978: finds by tag name 979: finds above another 980: finds child element 981: finds child element by tag name 982: finds elements with the shortcut syntax 983: raises if element not found 984: raises if invalid locator (FAILED - 1) ... 1014: is able to pass element arguments 1015: is able to pass in multiple arguments 1016: execute async script 1017: is able to return arrays of primitives from async scripts 1018: is able to pass multiple arguments to async scripts 1019: times out if the callback is not invoked 1020: Failures: 1021: 1) Selenium::WebDriver::Driver one element raises if invalid locator FIXED 1022: Expected pending 'Test guarded; Guarded by {:browser=>:edge, :platform=>:windows, :reason=>"https://bugs.chromium.org/p/chromedriver/issues/detail?id=4743"};' to fail. No error was raised. 1023: # ./rb/spec/integration/selenium/webdriver/driver_spec.rb:150 1024: Finished in 29.8 seconds (files took 0.6573 seconds to load) 1025: 51 examples, 1 failure 1026: Failed examples: ... 1052: finds by xpath 1053: finds by css selector 1054: finds by tag name 1055: finds above another 1056: finds child element 1057: finds child element by tag name 1058: finds elements with the shortcut syntax 1059: raises if element not found 1060: raises if invalid locator (FAILED - 1) ... 1090: is able to pass element arguments 1091: is able to pass in multiple arguments 1092: execute async script 1093: is able to return arrays of primitives from async scripts 1094: is able to pass multiple arguments to async scripts 1095: times out if the callback is not invoked 1096: Failures: 1097: 1) Selenium::WebDriver::Driver one element raises if invalid locator FIXED 1098: Expected pending 'Test guarded; Guarded by {:browser=>:edge, :platform=>:windows, :reason=>"https://bugs.chromium.org/p/chromedriver/issues/detail?id=4743"};' to fail. No error was raised. 1099: # ./rb/spec/integration/selenium/webdriver/driver_spec.rb:150 1100: Finished in 29.54 seconds (files took 0.70548 seconds to load) 1101: 51 examples, 1 failure 1102: Failed examples: ... 1128: finds by xpath 1129: finds by css selector 1130: finds by tag name 1131: finds above another 1132: finds child element 1133: finds child element by tag name 1134: finds elements with the shortcut syntax 1135: raises if element not found 1136: raises if invalid locator (FAILED - 1) ... 1166: is able to pass element arguments 1167: is able to pass in multiple arguments 1168: execute async script 1169: is able to return arrays of primitives from async scripts 1170: is able to pass multiple arguments to async scripts 1171: times out if the callback is not invoked 1172: Failures: 1173: 1) Selenium::WebDriver::Driver one element raises if invalid locator FIXED 1174: Expected pending 'Test guarded; Guarded by {:browser=>:edge, :platform=>:windows, :reason=>"https://bugs.chromium.org/p/chromedriver/issues/detail?id=4743"};' to fail. No error was raised. 1175: # ./rb/spec/integration/selenium/webdriver/driver_spec.rb:150 1176: Finished in 29.83 seconds (files took 0.58413 seconds to load) 1177: 51 examples, 1 failure 1178: Failed examples: 1179: rspec ./rb/spec/integration/selenium/webdriver/driver_spec.rb:150 # Selenium::WebDriver::Driver one element raises if invalid locator 1180: ================================================================================ 1181: [32m[3,126 / 3,128][0m 21 / 28 tests, [31m[1m1 failed[0m;[0m [Sched] Testing //rb/spec/integration/selenium/webdriver:element-edge-remote; 52s ... (2 actions, 1 running) 1182: [32m[3,126 / 3,128][0m 21 / 28 tests, [31m[1m1 failed[0m;[0m [Sched] Testing //rb/spec/integration/selenium/webdriver:element-edge-remote; 62s ... (2 actions, 1 running) 1183: [32m[3,126 / 3,128][0m 21 / 28 tests, [31m[1m1 failed[0m;[0m Testing //rb/spec/integration/selenium/webdriver:storage-edge-remote; 17s local, disk-cache ... (2 actions running) 1184: [32m[3,127 / 3,128][0m 22 / 28 tests, [31m[1m1 failed[0m;[0m Testing //rb/spec/integration/selenium/webdriver:element-edge-remote; 1s local, disk-cache 1185: [32m[3,127 / 3,128][0m 22 / 28 tests, [31m[1m1 failed[0m;[0m Testing //rb/spec/integration/selenium/webdriver:element-edge-remote; 11s local, disk-cache 1186: [32m[3,127 / 3,128][0m 22 / 28 tests, [31m[1m1 failed[0m;[0m Testing //rb/spec/integration/selenium/webdriver:element-edge-remote; 28s local, disk-cache 1187: [32m[3,128 / 3,129][0m 23 / 28 tests, [31m[1m1 failed[0m;[0m Testing //rb/spec/integration/selenium/webdriver/bidi:script-edge-remote; 0s disk-cache 1188: [32m[3,128 / 3,129][0m 23 / 28 tests, [31m[1m1 failed[0m;[0m [Sched] Testing //rb/spec/integration/selenium/webdriver/bidi:script-edge-remote 1189: [32m[3,128 / 3,129][0m 23 / 28 tests, [31m[1m1 failed[0m;[0m Testing //rb/spec/integration/selenium/webdriver/bidi:script-edge-remote; 1s local, disk-cache 1190: [32m[3,128 / 3,129][0m 23 / 28 tests, [31m[1m1 failed[0m;[0m Testing //rb/spec/integration/selenium/webdriver/bidi:script-edge-remote; 14s local, disk-cache 1191: [32m[3,129 / 3,130][0m 24 / 28 tests, [31m[1m1 failed[0m;[0m Testing //rb/spec/integration/selenium/webdriver:devtools-edge-remote; 0s disk-cache 1192: [32m[3,129 / 3,130][0m 24 / 28 tests, [31m[1m1 failed[0m;[0m [Sched] Testing //rb/spec/integration/selenium/webdriver:devtools-edge-remote 1193: [32m[3,129 / 3,130][0m 24 / 28 tests, [31m[1m1 failed[0m;[0m Testing //rb/spec/integration/selenium/webdriver:devtools-edge-remote; 1s local, disk-cache 1194: [32m[3,129 / 3,130][0m 24 / 28 tests, [31m[1m1 failed[0m;[0m Testing //rb/spec/integration/selenium/webdriver:devtools-edge-remote; 87s local, disk-cache 1195: [32m[3,130 / 3,131][0m 25 / 28 tests, [31m[1m1 failed[0m;[0m Testing //rb/spec/integration/selenium/webdriver/bidi:browsing_context-edge-remote; 1s disk-cache 1196: [32m[3,130 / 3,131][0m 25 / 28 tests, [31m[1m1 failed[0m;[0m [Sched] Testing //rb/spec/integration/selenium/webdriver/bidi:browsing_context-edge-remote 1197: [32m[3,130 / 3,131][0m 25 / 28 tests, [31m[1m1 failed[0m;[0m Testing //rb/spec/integration/selenium/webdriver/bidi:browsing_context-edge-remote; 1s local, disk-cache 1198: [32m[3,130 / 3,131][0m 25 / 28 tests, [31m[1m1 failed[0m;[0m Testing //rb/spec/integration/selenium/webdriver/bidi:browsing_context-edge-remote; 14s local, disk-cache 1199: [32m[3,131 / 3,132][0m 26 / 28 tests, [31m[1m1 failed[0m;[0m Testing //rb/spec/integration/selenium/webdriver/bidi:log_inspector-edge-remote; 0s disk-cache 1200: [32m[3,131 / 3,132][0m 26 / 28 tests, [31m[1m1 failed[0m;[0m [Sched] Testing //rb/spec/integration/selenium/webdriver/bidi:log_inspector-edge-remote 1201: [32m[3,131 / 3,132][0m 26 / 28 tests, [31m[1m1 failed[0m;[0m Testing //rb/spec/integration/selenium/webdriver/bidi:log_inspector-edge-remote; 1s local, disk-cache 1202: [32m[3,131 / 3,132][0m 26 / 28 tests, [31m[1m1 failed[0m;[0m Testing //rb/spec/integration/selenium/webdriver/bidi:log_inspector-edge-remote; 14s local, disk-cache 1203: [32m[3,132 / 3,133][0m 27 / 28 tests, [31m[1m1 failed[0m;[0m Testing //rb/spec/integration/selenium/webdriver:bidi-edge-remote; 1s disk-cache 1204: [32m[3,132 / 3,133][0m 27 / 28 tests, [31m[1m1 failed[0m;[0m [Sched] Testing //rb/spec/integration/selenium/webdriver:bidi-edge-remote 1205: [32m[3,132 / 3,133][0m 27 / 28 tests, [31m[1m1 failed[0m;[0m Testing //rb/spec/integration/selenium/webdriver:bidi-edge-remote; 1s local, disk-cache 1206: [32m[3,132 / 3,133][0m 27 / 28 tests, [31m[1m1 failed[0m;[0m Testing //rb/spec/integration/selenium/webdriver:bidi-edge-remote; 14s local, disk-cache 1207: [32mINFO: [0mFound 28 test targets... 1208: [32mINFO: [0mElapsed time: 1090.930s, Critical Path: 397.99s 1209: [32mINFO: [0m2895 processes: 1620 disk cache hit, 1189 internal, 85 local, 1 worker. 1210: [32mINFO: [0mBuild completed, 1 test FAILED, 2895 total actions 1211: //rb/spec/integration/selenium/webdriver:action_builder-edge-remote [0m[32mPASSED[0m in 27.8s 1212: //rb/spec/integration/selenium/webdriver:bidi-edge-remote [0m[32mPASSED[0m in 14.8s 1213: //rb/spec/integration/selenium/webdriver:devtools-edge-remote [0m[32mPASSED[0m in 87.5s 1214: //rb/spec/integration/selenium/webdriver:element-edge-remote [0m[32mPASSED[0m in 28.5s 1215: //rb/spec/integration/selenium/webdriver:error-edge-remote [0m[32mPASSED[0m in 15.7s ... 1230: //rb/spec/integration/selenium/webdriver/bidi:log_inspector-edge-remote [0m[32mPASSED[0m in 14.7s 1231: //rb/spec/integration/selenium/webdriver/bidi:script-edge-remote [0m[32mPASSED[0m in 14.3s 1232: //rb/spec/integration/selenium/webdriver/edge:driver-edge-remote [0m[32mPASSED[0m in 33.9s 1233: //rb/spec/integration/selenium/webdriver/edge:options-edge-remote [0m[32mPASSED[0m in 21.2s 1234: //rb/spec/integration/selenium/webdriver/edge:profile-edge-remote [0m[32mPASSED[0m in 14.7s 1235: //rb/spec/integration/selenium/webdriver/edge:service-edge-remote [0m[32mPASSED[0m in 21.5s 1236: //rb/spec/integration/selenium/webdriver/remote:driver-edge-remote [0m[32mPASSED[0m in 30.1s 1237: //rb/spec/integration/selenium/webdriver/remote:element-edge-remote [0m[32mPASSED[0m in 16.2s 1238: //rb/spec/integration/selenium/webdriver:driver-edge-remote [0m[31m[1mFAILED[0m in 3 out of 3 in 36.8s 1239: Stats over 3 runs: max = 36.8s, min = 36.5s, avg = 36.6s, dev = 0.1s 1240: D:/_bazel/execroot/_main/bazel-out/x64_windows-fastbuild/testlogs/rb/spec/integration/selenium/webdriver/driver-edge-remote/test.log 1241: D:/_bazel/execroot/_main/bazel-out/x64_windows-fastbuild/testlogs/rb/spec/integration/selenium/webdriver/driver-edge-remote/test_attempts/attempt_1.log 1242: D:/_bazel/execroot/_main/bazel-out/x64_windows-fastbuild/testlogs/rb/spec/integration/selenium/webdriver/driver-edge-remote/test_attempts/attempt_2.log 1243: Executed 28 out of 28 tests: 27 tests pass and [0m[31m[1m1 fails locally[0m. 1244: There were tests whose specified size is too big. Use the --test_verbose_timeout_warnings command line option to see which ones these are. 1245: [0m 1246: ##[error]Process completed with exit code 1. ``` |
Persistent review updated to latest commit https://github.com/SeleniumHQ/selenium/commit/465c31bac817c7d20369760fc7df3616402d0c24
Category | Suggestion | Score |
Possible bug |
Add a step to verify the extracted version is not empty before tagging the release___ **Add a step to verify that the extracted version is not empty before proceeding withtagging and pushing the release. This will prevent potential errors if the version extraction fails.** [.github/workflows/stage-release.yml [30-33]](https://github.com/SeleniumHQ/selenium/pull/14122/files#diff-7beae040710de3a234065211d66bf51c5bbfd1ccd6a8659966716fca0d955aa5R30-R33) ```diff +- name: Verify Version + run: | + if [ -z "${{ env.VERSION }}" ]; then + echo "Version extraction failed." + exit 1 + fi - name: Tag Release run: | git tag selenium-${{ env.VERSION }} git push origin selenium-${{ env.VERSION }} ``` - [ ] **Apply this suggestion** Suggestion importance[1-10]: 8Why: This is a valuable suggestion as it adds a verification step to ensure the version is correctly extracted before proceeding with critical operations like tagging and pushing, which can prevent errors in the release process. | 8 |
Possible issue |
Improve version extraction logic to handle branch names with multiple version-like patterns___ **Thegrep command used to extract the version from the branch name may not work correctly if the branch name contains multiple version-like patterns. Consider using a more specific regex or additional logic to ensure the correct version is extracted.** [.github/workflows/stage-release.yml [24]](https://github.com/SeleniumHQ/selenium/pull/14122/files#diff-7beae040710de3a234065211d66bf51c5bbfd1ccd6a8659966716fca0d955aa5R24-R24) ```diff -VERSION=$(echo $BRANCH_NAME | grep -oE '[0-9]+\.[0-9]+\.[0-9]+') +VERSION=$(echo $BRANCH_NAME | grep -oE 'release-preparation-[0-9]+\.[0-9]+\.[0-9]+' | grep -oE '[0-9]+\.[0-9]+\.[0-9]+') ``` - [ ] **Apply this suggestion** Suggestion importance[1-10]: 7Why: The suggestion correctly identifies a potential issue with the regex used for version extraction and provides a more specific regex to ensure accurate version extraction. | 7 |
Best practice |
Add a task dependency to ensure the
___
**Add a task dependency to ensure that the | 6 |
Well, this isn't everything I wanted it to be (description updated above), and I sure hope that --config=release works the way I hope it will. Merging.
User description
User description
./go all:release[--config=release]
everything has already been built and you're pretty much just downloading the top-level generated assets for publishing~I can't get it to complete on my fork because it won't do the RBE portion on my fork, and it is too late for me to spend more time on this tonight.~
~Hmm, it would make more sense to have a
all:package
task in the Rakefile so we don't have to call each of these individually. I'll update that.~We can theoretically have this "stage release" be a "full release", but this continues to require manual intervention for anything that can't be taken back. π
~We definitely want to add document generation and nightly version updates to this next, so we don't have to deal with that locally. Nightly update probably doesn't need manual intervention, but the API docs changes can be one big PR that gets reviewed before merging.... Anyway, that's next steps..~
Update
:docs
tasks in Rakefile, it isn't doing the complete flow with manual queries and pushes and checking out origin. It is just switching to gh-pages and making the commit. So if you're doing it manually you need to pay attention to it more.~Update 2
PR Type
Enhancement, Configuration changes
Description
stage-release.yml
) for release staging, which triggers on closed pull requests and handles building and packaging for multiple languages (Java, .NET, Ruby, Python, Node).Rakefile
by removing the creation of release notes and focusing on building and packaging.create_release_notes
task from theRakefile
.Changes walkthrough π
stage-release.yml
Add GitHub Actions workflow for release staging
.github/workflows/stage-release.yml
and Node projects.
Rakefile
Simplify release task and remove release notes generation
Rakefile
create_release_notes
task.PR Type
Enhancement, Configuration changes
Description
stage-release.yml
) to automate the release staging process. This workflow triggers on PR merge, tags the release, builds assets, creates a draft GitHub Release, and updates the nightly tag.Rakefile
to include a new taskall:package
for packaging artifacts. Modified theall:release
task to use this new packaging task. Removed thecreate_release_notes
task from theRakefile
.Changes walkthrough π
stage-release.yml
Add GitHub Actions workflow for release staging
.github/workflows/stage-release.yml
Rakefile
Update Rakefile with new packaging task and remove release notes task
Rakefile
all:package
to package artifacts.all:release
task to include the newall:package
task.create_release_notes
task.