From 8c317d89e6b88bb5a59c9e38a0485f3c15fa804e Mon Sep 17 00:00:00 2001 From: titusfortner Date: Sat, 22 Mar 2025 11:12:31 -0700 Subject: [PATCH 01/10] [build] allow tests tagged exclusive-if-local to run on rbe --- .bazelrc.remote | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.bazelrc.remote b/.bazelrc.remote index 40f864f95cda2..7b50bef8b48f1 100644 --- a/.bazelrc.remote +++ b/.bazelrc.remote @@ -31,7 +31,7 @@ build:remote --disk_cache= build:remote --incompatible_enable_cc_toolchain_resolution build:remote --action_env=BAZEL_DO_NOT_DETECT_CPP_TOOLCHAIN=1 test:remote --test_env=DISPLAY=:99.0 -test:remote --test_tag_filters=-exclusive-if-local,-skip-rbe,-remote +test:remote --test_tag_filters=-skip-rbe,-remote # Env vars we can hard code build:remote --action_env=HOME=/home/dev From 09fad560d1ac6dc8284b3d4f262c1ffab9e9931e Mon Sep 17 00:00:00 2001 From: aguspe Date: Sat, 24 May 2025 18:26:43 +0200 Subject: [PATCH 02/10] Add rescue and test for child_process terminate method --- .../webdriver/common/child_process.rb | 2 + .../webdriver/common/child_process_spec.rb | 38 +++++++++++++++++++ 2 files changed, 40 insertions(+) create mode 100644 rb/spec/unit/selenium/webdriver/common/child_process_spec.rb diff --git a/rb/lib/selenium/webdriver/common/child_process.rb b/rb/lib/selenium/webdriver/common/child_process.rb index 41fd206616da6..4980e27c8341d 100644 --- a/rb/lib/selenium/webdriver/common/child_process.rb +++ b/rb/lib/selenium/webdriver/common/child_process.rb @@ -122,6 +122,8 @@ def terminate_and_wait_else_kill(timeout) def terminate(pid) Process.kill(SIGTERM, pid) + rescue Errno::ECHILD, Errno::ESRCH + # Process does not exist, nothing to terminate end def kill(pid) diff --git a/rb/spec/unit/selenium/webdriver/common/child_process_spec.rb b/rb/spec/unit/selenium/webdriver/common/child_process_spec.rb new file mode 100644 index 0000000000000..d4a990de5df1c --- /dev/null +++ b/rb/spec/unit/selenium/webdriver/common/child_process_spec.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +# Licensed to the Software Freedom Conservancy (SFC) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The SFC licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +require File.expand_path('../spec_helper', __dir__) + +module Selenium + module WebDriver + describe ChildProcess do + it 'does not raise an error when terminating a killed process' do + process = described_class.new('sleep', '5') + process.start + + pid = process.instance_variable_get(:@pid) + Process.kill('KILL', pid) + + expect { + process.send(:terminate, pid) + }.not_to raise_error + end + end + end +end From 3c51e0bdbd95722c385ed45f9824a62e11205c0b Mon Sep 17 00:00:00 2001 From: aguspe Date: Sat, 24 May 2025 18:50:50 +0200 Subject: [PATCH 03/10] Add rescue for kill method --- .../selenium/webdriver/common/child_process.rb | 2 ++ .../webdriver/common/child_process_spec.rb | 16 +++++++++++++++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/rb/lib/selenium/webdriver/common/child_process.rb b/rb/lib/selenium/webdriver/common/child_process.rb index 4980e27c8341d..e30f82b6b28e8 100644 --- a/rb/lib/selenium/webdriver/common/child_process.rb +++ b/rb/lib/selenium/webdriver/common/child_process.rb @@ -128,6 +128,8 @@ def terminate(pid) def kill(pid) Process.kill(SIGKILL, pid) + rescue Errno::ECHILD, Errno::ESRCH + # Process does not exist, nothing to kill end def waitpid2(pid, flags = 0) diff --git a/rb/spec/unit/selenium/webdriver/common/child_process_spec.rb b/rb/spec/unit/selenium/webdriver/common/child_process_spec.rb index d4a990de5df1c..489aa6b2d793b 100644 --- a/rb/spec/unit/selenium/webdriver/common/child_process_spec.rb +++ b/rb/spec/unit/selenium/webdriver/common/child_process_spec.rb @@ -22,17 +22,31 @@ module Selenium module WebDriver describe ChildProcess do - it 'does not raise an error when terminating a killed process' do + it 'does not raise an error when terminating a non-existent process' do process = described_class.new('sleep', '5') process.start pid = process.instance_variable_get(:@pid) Process.kill('KILL', pid) + Process.wait(pid) expect { process.send(:terminate, pid) }.not_to raise_error end + + it 'does not raise an error when killing a non-existent process' do + process = described_class.new('sleep', '5') + process.start + + pid = process.instance_variable_get(:@pid) + Process.kill('KILL', pid) + Process.wait(pid) + + expect { + process.send(:kill, pid) + }.not_to raise_error + end end end end From 2d4107a1c496cb53520d0ae77d0201c2d6c7cf57 Mon Sep 17 00:00:00 2001 From: aguspe Date: Sat, 24 May 2025 19:39:02 +0200 Subject: [PATCH 04/10] guard against windows --- rb/spec/unit/selenium/webdriver/common/child_process_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rb/spec/unit/selenium/webdriver/common/child_process_spec.rb b/rb/spec/unit/selenium/webdriver/common/child_process_spec.rb index 489aa6b2d793b..31d2e7c5af6a9 100644 --- a/rb/spec/unit/selenium/webdriver/common/child_process_spec.rb +++ b/rb/spec/unit/selenium/webdriver/common/child_process_spec.rb @@ -21,7 +21,7 @@ module Selenium module WebDriver - describe ChildProcess do + describe ChildProcess, except: [{platform: :windows, reason: 'This is only for Unix platforms'}] do it 'does not raise an error when terminating a non-existent process' do process = described_class.new('sleep', '5') process.start From b86b31382b35ec94e9661df96b2f1c5d763f9a6d Mon Sep 17 00:00:00 2001 From: aguspe Date: Mon, 26 May 2025 20:05:44 +0200 Subject: [PATCH 05/10] guard against windows --- .../unit/selenium/webdriver/common/child_process_spec.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/rb/spec/unit/selenium/webdriver/common/child_process_spec.rb b/rb/spec/unit/selenium/webdriver/common/child_process_spec.rb index 31d2e7c5af6a9..4091021bb5ea7 100644 --- a/rb/spec/unit/selenium/webdriver/common/child_process_spec.rb +++ b/rb/spec/unit/selenium/webdriver/common/child_process_spec.rb @@ -21,8 +21,9 @@ module Selenium module WebDriver - describe ChildProcess, except: [{platform: :windows, reason: 'This is only for Unix platforms'}] do - it 'does not raise an error when terminating a non-existent process' do + describe ChildProcess do + it 'does not raise an error when terminating a non-existent process', + except: [{platform: :windows, reason: 'This is only for Unix platforms'}] do process = described_class.new('sleep', '5') process.start @@ -35,7 +36,8 @@ module WebDriver }.not_to raise_error end - it 'does not raise an error when killing a non-existent process' do + it 'does not raise an error when killing a non-existent process', + except: [{platform: :windows, reason: 'This is only for Unix platforms'}] do process = described_class.new('sleep', '5') process.start From 752fb7865cb332c1480cbfcc3ed4939e2c058b2f Mon Sep 17 00:00:00 2001 From: aguspe Date: Thu, 29 May 2025 23:03:35 +0200 Subject: [PATCH 06/10] move child process test --- .../selenium/webdriver}/child_process_spec.rb | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) rename rb/spec/{unit/selenium/webdriver/common => integration/selenium/webdriver}/child_process_spec.rb (85%) diff --git a/rb/spec/unit/selenium/webdriver/common/child_process_spec.rb b/rb/spec/integration/selenium/webdriver/child_process_spec.rb similarity index 85% rename from rb/spec/unit/selenium/webdriver/common/child_process_spec.rb rename to rb/spec/integration/selenium/webdriver/child_process_spec.rb index 4091021bb5ea7..94a605349325a 100644 --- a/rb/spec/unit/selenium/webdriver/common/child_process_spec.rb +++ b/rb/spec/integration/selenium/webdriver/child_process_spec.rb @@ -17,13 +17,12 @@ # specific language governing permissions and limitations # under the License. -require File.expand_path('../spec_helper', __dir__) +require_relative 'spec_helper' module Selenium module WebDriver - describe ChildProcess do - it 'does not raise an error when terminating a non-existent process', - except: [{platform: :windows, reason: 'This is only for Unix platforms'}] do + describe ChildProcess, except: [{platform: :windows, reason: 'This is only for Unix platforms'}] do + it 'does not raise an error when terminating a non-existent process' do process = described_class.new('sleep', '5') process.start @@ -36,8 +35,7 @@ module WebDriver }.not_to raise_error end - it 'does not raise an error when killing a non-existent process', - except: [{platform: :windows, reason: 'This is only for Unix platforms'}] do + it 'does not raise an error when killing a non-existent process' do process = described_class.new('sleep', '5') process.start From d4fdb8c537ca8262ee3aa4da0f9fa2dd86819982 Mon Sep 17 00:00:00 2001 From: aguspe Date: Fri, 30 May 2025 00:25:26 +0200 Subject: [PATCH 07/10] remove guard --- rb/spec/integration/selenium/webdriver/child_process_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rb/spec/integration/selenium/webdriver/child_process_spec.rb b/rb/spec/integration/selenium/webdriver/child_process_spec.rb index 94a605349325a..a31fd7173ab9b 100644 --- a/rb/spec/integration/selenium/webdriver/child_process_spec.rb +++ b/rb/spec/integration/selenium/webdriver/child_process_spec.rb @@ -21,7 +21,7 @@ module Selenium module WebDriver - describe ChildProcess, except: [{platform: :windows, reason: 'This is only for Unix platforms'}] do + describe ChildProcess do it 'does not raise an error when terminating a non-existent process' do process = described_class.new('sleep', '5') process.start From 869adb867092c44a4cdec9e5ceec1b0e8498f9f4 Mon Sep 17 00:00:00 2001 From: aguspe Date: Fri, 30 May 2025 13:28:00 +0200 Subject: [PATCH 08/10] Update test --- .../selenium/webdriver/common}/child_process_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename rb/spec/{integration/selenium/webdriver => unit/selenium/webdriver/common}/child_process_spec.rb (96%) diff --git a/rb/spec/integration/selenium/webdriver/child_process_spec.rb b/rb/spec/unit/selenium/webdriver/common/child_process_spec.rb similarity index 96% rename from rb/spec/integration/selenium/webdriver/child_process_spec.rb rename to rb/spec/unit/selenium/webdriver/common/child_process_spec.rb index a31fd7173ab9b..489aa6b2d793b 100644 --- a/rb/spec/integration/selenium/webdriver/child_process_spec.rb +++ b/rb/spec/unit/selenium/webdriver/common/child_process_spec.rb @@ -17,7 +17,7 @@ # specific language governing permissions and limitations # under the License. -require_relative 'spec_helper' +require File.expand_path('../spec_helper', __dir__) module Selenium module WebDriver From 71264c9c7d1633c9bf3c8ac054742e41b793183f Mon Sep 17 00:00:00 2001 From: aguspe Date: Fri, 30 May 2025 14:30:44 +0200 Subject: [PATCH 09/10] add windows guard --- rb/spec/unit/selenium/webdriver/common/child_process_spec.rb | 3 ++- rb/spec/unit/selenium/webdriver/spec_helper.rb | 5 ++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/rb/spec/unit/selenium/webdriver/common/child_process_spec.rb b/rb/spec/unit/selenium/webdriver/common/child_process_spec.rb index 489aa6b2d793b..18392a60d0762 100644 --- a/rb/spec/unit/selenium/webdriver/common/child_process_spec.rb +++ b/rb/spec/unit/selenium/webdriver/common/child_process_spec.rb @@ -21,7 +21,8 @@ module Selenium module WebDriver - describe ChildProcess do + describe ChildProcess, except: [{platform: :windows, + reason: 'This is only for Unix based systems'}] do it 'does not raise an error when terminating a non-existent process' do process = described_class.new('sleep', '5') process.start diff --git a/rb/spec/unit/selenium/webdriver/spec_helper.rb b/rb/spec/unit/selenium/webdriver/spec_helper.rb index cb6cf23d9b00d..44ee89c7538fb 100644 --- a/rb/spec/unit/selenium/webdriver/spec_helper.rb +++ b/rb/spec/unit/selenium/webdriver/spec_helper.rb @@ -33,6 +33,7 @@ require 'securerandom' require 'pathname' require_relative '../../../rspec_matchers' +require 'selenium/webdriver/support/guards' module Selenium module WebDriver @@ -62,7 +63,9 @@ def with_env(hash) c.run_all_when_everything_filtered = true c.default_formatter = c.files_to_run.count > 1 ? 'progress' : 'doc' - c.before do + c.before do |example| + guards = Selenium::WebDriver::Support::Guards.new(example, bug_tracker: 'https://github.com/SeleniumHQ/selenium/issues') + guards.add_condition(:platform, Selenium::WebDriver::Platform.os) # https://github.com/ruby/debug/issues/797 allow(File).to receive(:exist?).and_call_original end From 0ac3395fb97922adbc5376eeef229d3cfe5fdb2c Mon Sep 17 00:00:00 2001 From: aguspe Date: Fri, 30 May 2025 16:12:56 +0200 Subject: [PATCH 10/10] add windows guard --- .../webdriver/common/child_process_spec.rb | 53 ++++++++++--------- .../unit/selenium/webdriver/spec_helper.rb | 5 +- 2 files changed, 28 insertions(+), 30 deletions(-) diff --git a/rb/spec/unit/selenium/webdriver/common/child_process_spec.rb b/rb/spec/unit/selenium/webdriver/common/child_process_spec.rb index 18392a60d0762..fbec44b679ee1 100644 --- a/rb/spec/unit/selenium/webdriver/common/child_process_spec.rb +++ b/rb/spec/unit/selenium/webdriver/common/child_process_spec.rb @@ -21,32 +21,33 @@ module Selenium module WebDriver - describe ChildProcess, except: [{platform: :windows, - reason: 'This is only for Unix based systems'}] do - it 'does not raise an error when terminating a non-existent process' do - process = described_class.new('sleep', '5') - process.start - - pid = process.instance_variable_get(:@pid) - Process.kill('KILL', pid) - Process.wait(pid) - - expect { - process.send(:terminate, pid) - }.not_to raise_error - end - - it 'does not raise an error when killing a non-existent process' do - process = described_class.new('sleep', '5') - process.start - - pid = process.instance_variable_get(:@pid) - Process.kill('KILL', pid) - Process.wait(pid) - - expect { - process.send(:kill, pid) - }.not_to raise_error + describe ChildProcess do + unless Selenium::WebDriver::Platform.windows? + it 'does not raise an error when terminating a non-existent process' do + process = described_class.new('sleep', '5') + process.start + + pid = process.instance_variable_get(:@pid) + Process.kill('KILL', pid) + Process.wait(pid) + + expect { + process.send(:terminate, pid) + }.not_to raise_error + end + + it 'does not raise an error when killing a non-existent process' do + process = described_class.new('sleep', '5') + process.start + + pid = process.instance_variable_get(:@pid) + Process.kill('KILL', pid) + Process.wait(pid) + + expect { + process.send(:kill, pid) + }.not_to raise_error + end end end end diff --git a/rb/spec/unit/selenium/webdriver/spec_helper.rb b/rb/spec/unit/selenium/webdriver/spec_helper.rb index 44ee89c7538fb..cb6cf23d9b00d 100644 --- a/rb/spec/unit/selenium/webdriver/spec_helper.rb +++ b/rb/spec/unit/selenium/webdriver/spec_helper.rb @@ -33,7 +33,6 @@ require 'securerandom' require 'pathname' require_relative '../../../rspec_matchers' -require 'selenium/webdriver/support/guards' module Selenium module WebDriver @@ -63,9 +62,7 @@ def with_env(hash) c.run_all_when_everything_filtered = true c.default_formatter = c.files_to_run.count > 1 ? 'progress' : 'doc' - c.before do |example| - guards = Selenium::WebDriver::Support::Guards.new(example, bug_tracker: 'https://github.com/SeleniumHQ/selenium/issues') - guards.add_condition(:platform, Selenium::WebDriver::Platform.os) + c.before do # https://github.com/ruby/debug/issues/797 allow(File).to receive(:exist?).and_call_original end