From ec5e310abc56cc8f31c1e889dc1a50408355d426 Mon Sep 17 00:00:00 2001 From: John Bachir Date: Fri, 30 Jun 2023 19:49:58 -0400 Subject: [PATCH 01/27] error lifecycle tests --- test/lib/error_lifecycle.rb | 47 +++++++++++++++ test/test_error_lifecycle.rb | 113 +++++++++++++++++++++++++++++++++++ 2 files changed, 160 insertions(+) create mode 100644 test/lib/error_lifecycle.rb create mode 100644 test/test_error_lifecycle.rb diff --git a/test/lib/error_lifecycle.rb b/test/lib/error_lifecycle.rb new file mode 100644 index 0000000..96159bc --- /dev/null +++ b/test/lib/error_lifecycle.rb @@ -0,0 +1,47 @@ +class MyStandardError < StandardError; end +class MyException< Exception; end + +def subject(error_to_raise, error_to_rescue) + $inner_attempted = nil + $inner_else = nil + $inner_rescue = nil + $inner_ensure = nil + $inner_ensure_has_time_to_finish = nil + + $outer_rescue = nil + $outer_else = nil + $outer_ensure = nil + $outer_ensure_has_time_to_finish = nil + + begin + Timeout.timeout(0.001, error_to_raise){ + begin + $inner_attempted = true + nil while true + rescue error_to_rescue + $inner_rescue = true + else + $inner_else = true + ensure + $inner_ensure = true + t = Time.now; nil while Time.now < t+1 + $inner_ensure_has_time_to_finish = true + end + } + rescue Exception + $outer_rescue = true + else + $outer_else = true + ensure + $outer_ensure = true + t = Time.now; nil while Time.now < t+1 + $outer_ensure_has_time_to_finish = true + end + + # remove if/when stop using global variables + Thread.list.each{|t| t.kill unless t==Thread.current } + + unless !!$outer_else ^ !!$outer_rescue + raise "something strange happened with the outer_rescue variables" + end +end diff --git a/test/test_error_lifecycle.rb b/test/test_error_lifecycle.rb new file mode 100644 index 0000000..d0f970d --- /dev/null +++ b/test/test_error_lifecycle.rb @@ -0,0 +1,113 @@ +require 'test/unit' +require 'timeout' +require 'thread' + +class TestTimeout < Test::Unit::TestCase + + ### Tests demonstrating problems with standard lib + # NOTE: this demonstration was done such that all of the assertions pass, + # The ones marked weird, bad, and very bad should not, and their + # passing is demonstrating the brokenness. + # ruby gem at 0f12a0ec11d4a860a56e74a2bb051a77fe70b006 also passes + + require_relative 'lib/error_lifecycle.rb' + + # when an exception to raise is not specified and the inner code does not catch Exception + def test_1 + subject(nil, StandardError) + + # EXPECTED + assert $inner_attempted + assert !$inner_else + assert !$inner_rescue + assert $inner_ensure + assert $outer_rescue + assert $outer_ensure + assert $inner_ensure_has_time_to_finish + assert $outer_ensure_has_time_to_finish + end + + # when an exception to raise is not specified and the inner code does catch Exception + def test_2 + subject(nil, Exception) + + # EXPECTED + assert $inner_attempted + assert !$inner_else + assert $inner_ensure + assert $outer_ensure + assert $inner_ensure_has_time_to_finish + assert $outer_ensure_has_time_to_finish + assert $inner_rescue # true in 1.9, false in gem 0.2.0, true in 0.4.0 + + # BAD? + assert !$outer_rescue # false in 1.9 stdlib, true in gem 0.2.0, false in 0.4.0 + end + + # when an exception to raise is StandardError and the inner code does not catch Exception + def test_3 + subject(MyStandardError, StandardError) + + # EXPECTED + assert $inner_attempted + assert !$inner_else + assert $inner_rescue + assert $inner_ensure + assert $outer_ensure + assert $inner_ensure_has_time_to_finish + assert $outer_ensure_has_time_to_finish + + # BAD? + assert !$outer_rescue + end + + # when an exception to raise is StandardError and the inner code does catch Exception + def test_4 + subject(MyStandardError, Exception) + + # EXPECTED + assert $inner_attempted + assert !$inner_else + assert $inner_rescue + assert $inner_ensure + assert $outer_ensure + assert $inner_ensure_has_time_to_finish + assert $outer_ensure_has_time_to_finish + + # BAD? + assert !$outer_rescue + end + + # when an exception to raise is Exception and the inner code does not catch Exception + def test_5 + subject(MyException, StandardError) + + # EXPECTED + assert $inner_attempted + assert !$inner_else + assert !$inner_rescue + assert $inner_ensure + assert $outer_ensure + assert $outer_rescue + assert $inner_ensure_has_time_to_finish + assert $outer_ensure_has_time_to_finish + end + + # when an exception to raise is Exception and the inner code does catch Exception + def test_6 + subject(MyException, Exception) + + # EXPECTED + assert $inner_attempted + assert !$inner_else + assert $inner_rescue + assert $inner_ensure + assert $outer_ensure + assert $inner_ensure_has_time_to_finish + assert $outer_ensure_has_time_to_finish + + # BAD? + assert !$outer_rescue + end + +end From e7ae1b34b274269ba5a09cedc1a5d57d9d6660d2 Mon Sep 17 00:00:00 2001 From: John Bachir Date: Fri, 30 Jun 2023 20:25:52 -0400 Subject: [PATCH 02/27] no more globals! --- test/lib/error_lifecycle.rb | 80 ++++++++++++------------ test/test_error_lifecycle.rb | 114 ++++++++++++++++++----------------- 2 files changed, 101 insertions(+), 93 deletions(-) diff --git a/test/lib/error_lifecycle.rb b/test/lib/error_lifecycle.rb index 96159bc..bc0fa8a 100644 --- a/test/lib/error_lifecycle.rb +++ b/test/lib/error_lifecycle.rb @@ -1,47 +1,49 @@ class MyStandardError < StandardError; end class MyException< Exception; end -def subject(error_to_raise, error_to_rescue) - $inner_attempted = nil - $inner_else = nil - $inner_rescue = nil - $inner_ensure = nil - $inner_ensure_has_time_to_finish = nil +class ErrorLifeCycleTester + attr_reader :inner_attempted, :inner_else, :inner_rescue, :inner_ensure, :inner_ensure_has_time_to_finish, :outer_rescue, :outer_else, :outer_ensure, :outer_ensure_has_time_to_finish - $outer_rescue = nil - $outer_else = nil - $outer_ensure = nil - $outer_ensure_has_time_to_finish = nil + def subject(error_to_raise, error_to_rescue) - begin - Timeout.timeout(0.001, error_to_raise){ - begin - $inner_attempted = true - nil while true - rescue error_to_rescue - $inner_rescue = true - else - $inner_else = true - ensure - $inner_ensure = true - t = Time.now; nil while Time.now < t+1 - $inner_ensure_has_time_to_finish = true - end - } - rescue Exception - $outer_rescue = true - else - $outer_else = true - ensure - $outer_ensure = true - t = Time.now; nil while Time.now < t+1 - $outer_ensure_has_time_to_finish = true - end + @inner_attempted = nil + @inner_else = nil + @inner_rescue = nil + @inner_ensure = nil + @inner_ensure_has_time_to_finish = nil + + @outer_rescue = nil + @outer_else = nil + @outer_ensure = nil + @outer_ensure_has_time_to_finish = nil - # remove if/when stop using global variables - Thread.list.each{|t| t.kill unless t==Thread.current } + begin + Timeout.timeout(0.001, error_to_raise){ + begin + @inner_attempted = true + nil while true + rescue error_to_rescue + @inner_rescue = true + else + @inner_else = true + ensure + @inner_ensure = true + t = Time.now; nil while Time.now < t+1 + @inner_ensure_has_time_to_finish = true + end + } + rescue Exception + @outer_rescue = true + else + @outer_else = true + ensure + @outer_ensure = true + t = Time.now; nil while Time.now < t+1 + @outer_ensure_has_time_to_finish = true + end - unless !!$outer_else ^ !!$outer_rescue - raise "something strange happened with the outer_rescue variables" + unless !!@outer_else ^ !!@outer_rescue + raise "something strange happened with the outer_rescue variables" + end end -end +end \ No newline at end of file diff --git a/test/test_error_lifecycle.rb b/test/test_error_lifecycle.rb index d0f970d..710c5ae 100644 --- a/test/test_error_lifecycle.rb +++ b/test/test_error_lifecycle.rb @@ -14,100 +14,106 @@ class TestTimeout < Test::Unit::TestCase # when an exception to raise is not specified and the inner code does not catch Exception def test_1 - subject(nil, StandardError) + s = ErrorLifeCycleTester.new + s.subject(nil, StandardError) # EXPECTED - assert $inner_attempted - assert !$inner_else - assert !$inner_rescue - assert $inner_ensure - assert $outer_rescue - assert $outer_ensure - assert $inner_ensure_has_time_to_finish - assert $outer_ensure_has_time_to_finish + assert s.inner_attempted + assert !s.inner_else + assert !s.inner_rescue + assert s.inner_ensure + assert s.outer_rescue + assert s.outer_ensure + assert s.inner_ensure_has_time_to_finish + assert s.outer_ensure_has_time_to_finish end # when an exception to raise is not specified and the inner code does catch Exception def test_2 - subject(nil, Exception) + s = ErrorLifeCycleTester.new + s.subject(nil, Exception) # EXPECTED - assert $inner_attempted - assert !$inner_else - assert $inner_ensure - assert $outer_ensure - assert $inner_ensure_has_time_to_finish - assert $outer_ensure_has_time_to_finish - assert $inner_rescue # true in 1.9, false in gem 0.2.0, true in 0.4.0 + assert s.inner_attempted + assert !s.inner_else + assert s.inner_ensure + assert s.outer_ensure + assert s.inner_ensure_has_time_to_finish + assert s.outer_ensure_has_time_to_finish + assert s.inner_rescue # true in 1.9, false in gem 0.2.0, true in 0.4.0 # BAD? - assert !$outer_rescue # false in 1.9 stdlib, true in gem 0.2.0, false in 0.4.0 + assert !s.outer_rescue # false in 1.9 stdlib, true in gem 0.2.0, false in 0.4.0 end # when an exception to raise is StandardError and the inner code does not catch Exception def test_3 - subject(MyStandardError, StandardError) + s = ErrorLifeCycleTester.new + s.subject(MyStandardError, StandardError) # EXPECTED - assert $inner_attempted - assert !$inner_else - assert $inner_rescue - assert $inner_ensure - assert $outer_ensure - assert $inner_ensure_has_time_to_finish - assert $outer_ensure_has_time_to_finish + assert s.inner_attempted + assert !s.inner_else + assert s.inner_rescue + assert s.inner_ensure + assert s.outer_ensure + assert s.inner_ensure_has_time_to_finish + assert s.outer_ensure_has_time_to_finish # BAD? - assert !$outer_rescue + assert !s.outer_rescue end # when an exception to raise is StandardError and the inner code does catch Exception def test_4 - subject(MyStandardError, Exception) + s = ErrorLifeCycleTester.new + s.subject(MyStandardError, Exception) # EXPECTED - assert $inner_attempted - assert !$inner_else - assert $inner_rescue - assert $inner_ensure - assert $outer_ensure - assert $inner_ensure_has_time_to_finish - assert $outer_ensure_has_time_to_finish + assert s.inner_attempted + assert !s.inner_else + assert s.inner_rescue + assert s.inner_ensure + assert s.outer_ensure + assert s.inner_ensure_has_time_to_finish + assert s.outer_ensure_has_time_to_finish # BAD? - assert !$outer_rescue + assert !s.outer_rescue end # when an exception to raise is Exception and the inner code does not catch Exception def test_5 - subject(MyException, StandardError) + s = ErrorLifeCycleTester.new + s.subject(MyException, StandardError) # EXPECTED - assert $inner_attempted - assert !$inner_else - assert !$inner_rescue - assert $inner_ensure - assert $outer_ensure - assert $outer_rescue - assert $inner_ensure_has_time_to_finish - assert $outer_ensure_has_time_to_finish + assert s.inner_attempted + assert !s.inner_else + assert !s.inner_rescue + assert s.inner_ensure + assert s.outer_ensure + assert s.outer_rescue + assert s.inner_ensure_has_time_to_finish + assert s.outer_ensure_has_time_to_finish end # when an exception to raise is Exception and the inner code does catch Exception def test_6 - subject(MyException, Exception) + s = ErrorLifeCycleTester.new + s.subject(MyException, Exception) # EXPECTED - assert $inner_attempted - assert !$inner_else - assert $inner_rescue - assert $inner_ensure - assert $outer_ensure - assert $inner_ensure_has_time_to_finish - assert $outer_ensure_has_time_to_finish + assert s.inner_attempted + assert !s.inner_else + assert s.inner_rescue + assert s.inner_ensure + assert s.outer_ensure + assert s.inner_ensure_has_time_to_finish + assert s.outer_ensure_has_time_to_finish # BAD? - assert !$outer_rescue + assert !s.outer_rescue end end From 096d15e3f826e167eeab5273a8da3315835c733f Mon Sep 17 00:00:00 2001 From: John Bachir Date: Fri, 30 Jun 2023 20:32:34 -0400 Subject: [PATCH 03/27] whitespace --- test/lib/error_lifecycle.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/lib/error_lifecycle.rb b/test/lib/error_lifecycle.rb index bc0fa8a..99df765 100644 --- a/test/lib/error_lifecycle.rb +++ b/test/lib/error_lifecycle.rb @@ -46,4 +46,4 @@ def subject(error_to_raise, error_to_rescue) raise "something strange happened with the outer_rescue variables" end end -end \ No newline at end of file +end From fbd0e644771f1a2d21f74c43be190a81b536f923 Mon Sep 17 00:00:00 2001 From: John Bachir Date: Fri, 30 Jun 2023 20:41:39 -0400 Subject: [PATCH 04/27] comments --- test/test_error_lifecycle.rb | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/test/test_error_lifecycle.rb b/test/test_error_lifecycle.rb index 710c5ae..f2a2c09 100644 --- a/test/test_error_lifecycle.rb +++ b/test/test_error_lifecycle.rb @@ -4,12 +4,8 @@ class TestTimeout < Test::Unit::TestCase - ### Tests demonstrating problems with standard lib - # NOTE: this demonstration was done such that all of the assertions pass, - # The ones marked weird, bad, and very bad should not, and their - # passing is demonstrating the brokenness. - # ruby gem at 0f12a0ec11d4a860a56e74a2bb051a77fe70b006 also passes - + # Behavior marked "BAD?" is done so as John's opinion, these can/should be removed before the PR is merged + require_relative 'lib/error_lifecycle.rb' # when an exception to raise is not specified and the inner code does not catch Exception From d0fb42457c0389e5e89843fd140f9037e18a1f75 Mon Sep 17 00:00:00 2001 From: John Bachir Date: Fri, 30 Jun 2023 20:44:25 -0400 Subject: [PATCH 05/27] code improvement --- test/lib/error_lifecycle.rb | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/test/lib/error_lifecycle.rb b/test/lib/error_lifecycle.rb index 99df765..c6b4b28 100644 --- a/test/lib/error_lifecycle.rb +++ b/test/lib/error_lifecycle.rb @@ -18,20 +18,18 @@ def subject(error_to_raise, error_to_rescue) @outer_ensure_has_time_to_finish = nil begin - Timeout.timeout(0.001, error_to_raise){ - begin - @inner_attempted = true - nil while true - rescue error_to_rescue - @inner_rescue = true - else - @inner_else = true - ensure - @inner_ensure = true - t = Time.now; nil while Time.now < t+1 - @inner_ensure_has_time_to_finish = true - end - } + Timeout.timeout(0.001, error_to_raise) do + @inner_attempted = true + nil while true + rescue error_to_rescue + @inner_rescue = true + else + @inner_else = true + ensure + @inner_ensure = true + t = Time.now; nil while Time.now < t+1 + @inner_ensure_has_time_to_finish = true + end rescue Exception @outer_rescue = true else From d42ef28c89f4a5303e2e61f73a276c25671d089b Mon Sep 17 00:00:00 2001 From: John Bachir Date: Fri, 30 Jun 2023 21:15:30 -0400 Subject: [PATCH 06/27] comments --- test/test_error_lifecycle.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/test_error_lifecycle.rb b/test/test_error_lifecycle.rb index f2a2c09..7be9218 100644 --- a/test/test_error_lifecycle.rb +++ b/test/test_error_lifecycle.rb @@ -4,8 +4,8 @@ class TestTimeout < Test::Unit::TestCase - # Behavior marked "BAD?" is done so as John's opinion, these can/should be removed before the PR is merged - + # Behavior marked "UNDESIRED?" is done so as John's opinion, these can/should be removed before the PR is merged + require_relative 'lib/error_lifecycle.rb' # when an exception to raise is not specified and the inner code does not catch Exception @@ -38,7 +38,7 @@ def test_2 assert s.outer_ensure_has_time_to_finish assert s.inner_rescue # true in 1.9, false in gem 0.2.0, true in 0.4.0 - # BAD? + # UNDESIRED? assert !s.outer_rescue # false in 1.9 stdlib, true in gem 0.2.0, false in 0.4.0 end @@ -56,7 +56,7 @@ def test_3 assert s.inner_ensure_has_time_to_finish assert s.outer_ensure_has_time_to_finish - # BAD? + # UNDESIRED? assert !s.outer_rescue end @@ -74,7 +74,7 @@ def test_4 assert s.inner_ensure_has_time_to_finish assert s.outer_ensure_has_time_to_finish - # BAD? + # UNDESIRED? assert !s.outer_rescue end @@ -108,7 +108,7 @@ def test_6 assert s.inner_ensure_has_time_to_finish assert s.outer_ensure_has_time_to_finish - # BAD? + # UNDESIRED? assert !s.outer_rescue end From cb1ef51ce6e5edbdf1b9968f9b4acc7761468fec Mon Sep 17 00:00:00 2001 From: John Bachir Date: Sat, 1 Jul 2023 00:34:19 -0400 Subject: [PATCH 07/27] move support file out of lib because it should only be used for extending test-unit --- test/{lib => }/error_lifecycle.rb | 0 test/test_error_lifecycle.rb | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) rename test/{lib => }/error_lifecycle.rb (100%) diff --git a/test/lib/error_lifecycle.rb b/test/error_lifecycle.rb similarity index 100% rename from test/lib/error_lifecycle.rb rename to test/error_lifecycle.rb diff --git a/test/test_error_lifecycle.rb b/test/test_error_lifecycle.rb index 7be9218..2dc5683 100644 --- a/test/test_error_lifecycle.rb +++ b/test/test_error_lifecycle.rb @@ -6,7 +6,7 @@ class TestTimeout < Test::Unit::TestCase # Behavior marked "UNDESIRED?" is done so as John's opinion, these can/should be removed before the PR is merged - require_relative 'lib/error_lifecycle.rb' + require_relative 'error_lifecycle.rb' # when an exception to raise is not specified and the inner code does not catch Exception def test_1 From 1d4a8a86709aafd087d7630018ead86d3c0c1a0a Mon Sep 17 00:00:00 2001 From: John Bachir Date: Sat, 1 Jul 2023 00:37:19 -0400 Subject: [PATCH 08/27] formatting --- test/error_lifecycle.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/error_lifecycle.rb b/test/error_lifecycle.rb index c6b4b28..7a6c522 100644 --- a/test/error_lifecycle.rb +++ b/test/error_lifecycle.rb @@ -2,10 +2,10 @@ class MyStandardError < StandardError; end class MyException< Exception; end class ErrorLifeCycleTester - attr_reader :inner_attempted, :inner_else, :inner_rescue, :inner_ensure, :inner_ensure_has_time_to_finish, :outer_rescue, :outer_else, :outer_ensure, :outer_ensure_has_time_to_finish + attr_reader :inner_attempted, :inner_else, :inner_rescue, :inner_ensure, :inner_ensure_has_time_to_finish, + :outer_rescue, :outer_else, :outer_ensure, :outer_ensure_has_time_to_finish def subject(error_to_raise, error_to_rescue) - @inner_attempted = nil @inner_else = nil @inner_rescue = nil From dd46da03363f48443970927a4e6fdcc96df780e1 Mon Sep 17 00:00:00 2001 From: John Bachir Date: Sat, 1 Jul 2023 00:46:14 -0400 Subject: [PATCH 09/27] cleanup --- test/test_error_lifecycle.rb | 6 ------ 1 file changed, 6 deletions(-) diff --git a/test/test_error_lifecycle.rb b/test/test_error_lifecycle.rb index 2dc5683..648352f 100644 --- a/test/test_error_lifecycle.rb +++ b/test/test_error_lifecycle.rb @@ -13,7 +13,6 @@ def test_1 s = ErrorLifeCycleTester.new s.subject(nil, StandardError) - # EXPECTED assert s.inner_attempted assert !s.inner_else assert !s.inner_rescue @@ -29,7 +28,6 @@ def test_2 s = ErrorLifeCycleTester.new s.subject(nil, Exception) - # EXPECTED assert s.inner_attempted assert !s.inner_else assert s.inner_ensure @@ -47,7 +45,6 @@ def test_3 s = ErrorLifeCycleTester.new s.subject(MyStandardError, StandardError) - # EXPECTED assert s.inner_attempted assert !s.inner_else assert s.inner_rescue @@ -65,7 +62,6 @@ def test_4 s = ErrorLifeCycleTester.new s.subject(MyStandardError, Exception) - # EXPECTED assert s.inner_attempted assert !s.inner_else assert s.inner_rescue @@ -83,7 +79,6 @@ def test_5 s = ErrorLifeCycleTester.new s.subject(MyException, StandardError) - # EXPECTED assert s.inner_attempted assert !s.inner_else assert !s.inner_rescue @@ -99,7 +94,6 @@ def test_6 s = ErrorLifeCycleTester.new s.subject(MyException, Exception) - # EXPECTED assert s.inner_attempted assert !s.inner_else assert s.inner_rescue From 9da21eeed2c2a7ea2f87703d3440d6a14e01ac48 Mon Sep 17 00:00:00 2001 From: John Bachir Date: Sat, 1 Jul 2023 12:43:56 -0400 Subject: [PATCH 10/27] DRY --- test/test_error_lifecycle.rb | 53 +++++++++++------------------------- 1 file changed, 16 insertions(+), 37 deletions(-) diff --git a/test/test_error_lifecycle.rb b/test/test_error_lifecycle.rb index 648352f..dbc59e6 100644 --- a/test/test_error_lifecycle.rb +++ b/test/test_error_lifecycle.rb @@ -8,32 +8,31 @@ class TestTimeout < Test::Unit::TestCase require_relative 'error_lifecycle.rb' + def core_assertions(s) + assert s.inner_attempted + assert !s.inner_else + assert s.inner_ensure + assert s.outer_ensure + assert s.inner_ensure_has_time_to_finish + assert s.outer_ensure_has_time_to_finish + end + # when an exception to raise is not specified and the inner code does not catch Exception def test_1 s = ErrorLifeCycleTester.new s.subject(nil, StandardError) + core_assertions(s) - assert s.inner_attempted - assert !s.inner_else assert !s.inner_rescue - assert s.inner_ensure assert s.outer_rescue - assert s.outer_ensure - assert s.inner_ensure_has_time_to_finish - assert s.outer_ensure_has_time_to_finish end # when an exception to raise is not specified and the inner code does catch Exception def test_2 s = ErrorLifeCycleTester.new s.subject(nil, Exception) + core_assertions(s) - assert s.inner_attempted - assert !s.inner_else - assert s.inner_ensure - assert s.outer_ensure - assert s.inner_ensure_has_time_to_finish - assert s.outer_ensure_has_time_to_finish assert s.inner_rescue # true in 1.9, false in gem 0.2.0, true in 0.4.0 # UNDESIRED? @@ -44,14 +43,9 @@ def test_2 def test_3 s = ErrorLifeCycleTester.new s.subject(MyStandardError, StandardError) + core_assertions(s) - assert s.inner_attempted - assert !s.inner_else assert s.inner_rescue - assert s.inner_ensure - assert s.outer_ensure - assert s.inner_ensure_has_time_to_finish - assert s.outer_ensure_has_time_to_finish # UNDESIRED? assert !s.outer_rescue @@ -61,14 +55,9 @@ def test_3 def test_4 s = ErrorLifeCycleTester.new s.subject(MyStandardError, Exception) + core_assertions(s) - assert s.inner_attempted - assert !s.inner_else assert s.inner_rescue - assert s.inner_ensure - assert s.outer_ensure - assert s.inner_ensure_has_time_to_finish - assert s.outer_ensure_has_time_to_finish # UNDESIRED? assert !s.outer_rescue @@ -78,29 +67,19 @@ def test_4 def test_5 s = ErrorLifeCycleTester.new s.subject(MyException, StandardError) + core_assertions(s) - assert s.inner_attempted - assert !s.inner_else assert !s.inner_rescue - assert s.inner_ensure - assert s.outer_ensure assert s.outer_rescue - assert s.inner_ensure_has_time_to_finish - assert s.outer_ensure_has_time_to_finish end # when an exception to raise is Exception and the inner code does catch Exception def test_6 s = ErrorLifeCycleTester.new s.subject(MyException, Exception) - - assert s.inner_attempted - assert !s.inner_else + core_assertions(s) + assert s.inner_rescue - assert s.inner_ensure - assert s.outer_ensure - assert s.inner_ensure_has_time_to_finish - assert s.outer_ensure_has_time_to_finish # UNDESIRED? assert !s.outer_rescue From 950c0db93447ab270902f6335a60a1db5eaf9ae2 Mon Sep 17 00:00:00 2001 From: John Bachir Date: Sat, 1 Jul 2023 12:47:56 -0400 Subject: [PATCH 11/27] note about ensure time --- test/test_error_lifecycle.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/test_error_lifecycle.rb b/test/test_error_lifecycle.rb index dbc59e6..f97a7ab 100644 --- a/test/test_error_lifecycle.rb +++ b/test/test_error_lifecycle.rb @@ -13,6 +13,11 @@ def core_assertions(s) assert !s.inner_else assert s.inner_ensure assert s.outer_ensure + + # This can result in user's expectation of total possible time + # being very wrong + # t = Time.now; Timeout.timeout(0.1){begin; sleep 1; ensure; sleep 2; end} rescue puts Time.now-t + # => 2.106306 assert s.inner_ensure_has_time_to_finish assert s.outer_ensure_has_time_to_finish end From abc3a6b9cdd12ce1a8ba92cbfdfc80a1d99f357f Mon Sep 17 00:00:00 2001 From: John Bachir Date: Sat, 1 Jul 2023 14:14:33 -0400 Subject: [PATCH 12/27] comment --- test/error_lifecycle.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/error_lifecycle.rb b/test/error_lifecycle.rb index 7a6c522..3892a42 100644 --- a/test/error_lifecycle.rb +++ b/test/error_lifecycle.rb @@ -40,6 +40,8 @@ def subject(error_to_raise, error_to_rescue) @outer_ensure_has_time_to_finish = true end + # this is here to avoid cluttering the "UNDESIRED?" section of each test, + # can be flatted into the main tests unless !!@outer_else ^ !!@outer_rescue raise "something strange happened with the outer_rescue variables" end From 9b9c26cdee9464faf907edec4bd1ef00bca5818c Mon Sep 17 00:00:00 2001 From: John Bachir Date: Sat, 1 Jul 2023 14:15:13 -0400 Subject: [PATCH 13/27] false instead of nil --- test/error_lifecycle.rb | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/test/error_lifecycle.rb b/test/error_lifecycle.rb index 3892a42..db89cd5 100644 --- a/test/error_lifecycle.rb +++ b/test/error_lifecycle.rb @@ -6,16 +6,16 @@ class ErrorLifeCycleTester :outer_rescue, :outer_else, :outer_ensure, :outer_ensure_has_time_to_finish def subject(error_to_raise, error_to_rescue) - @inner_attempted = nil - @inner_else = nil - @inner_rescue = nil - @inner_ensure = nil - @inner_ensure_has_time_to_finish = nil + @inner_attempted = false + @inner_else = false + @inner_rescue = false + @inner_ensure = false + @inner_ensure_has_time_to_finish = false - @outer_rescue = nil - @outer_else = nil - @outer_ensure = nil - @outer_ensure_has_time_to_finish = nil + @outer_rescue = false + @outer_else = false + @outer_ensure = false + @outer_ensure_has_time_to_finish = false begin Timeout.timeout(0.001, error_to_raise) do @@ -42,7 +42,7 @@ def subject(error_to_raise, error_to_rescue) # this is here to avoid cluttering the "UNDESIRED?" section of each test, # can be flatted into the main tests - unless !!@outer_else ^ !!@outer_rescue + unless @outer_else ^ @outer_rescue raise "something strange happened with the outer_rescue variables" end end From b3bec3f14a2e97e6e55ed269686a5ca901c6f686 Mon Sep 17 00:00:00 2001 From: John Bachir Date: Sat, 1 Jul 2023 14:17:14 -0400 Subject: [PATCH 14/27] comment --- test/test_error_lifecycle.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/test_error_lifecycle.rb b/test/test_error_lifecycle.rb index f97a7ab..c20670f 100644 --- a/test/test_error_lifecycle.rb +++ b/test/test_error_lifecycle.rb @@ -38,10 +38,10 @@ def test_2 s.subject(nil, Exception) core_assertions(s) - assert s.inner_rescue # true in 1.9, false in gem 0.2.0, true in 0.4.0 + assert s.inner_rescue # true in 1.9, false in gem 0.2.0, true in gem 0.4.0 # UNDESIRED? - assert !s.outer_rescue # false in 1.9 stdlib, true in gem 0.2.0, false in 0.4.0 + assert !s.outer_rescue # false in 1.9 stdlib, true in gem 0.2.0, false in gem 0.4.0 end # when an exception to raise is StandardError and the inner code does not catch Exception From 6e52e0022a2c291e3b9beaf1ed65e6f9af7aef5e Mon Sep 17 00:00:00 2001 From: John Bachir Date: Sat, 1 Jul 2023 12:31:49 -0400 Subject: [PATCH 15/27] wip --- lib/timeout.rb | 37 +++++++++++++++++++++++++++++++++--- test/error_lifecycle.rb | 2 +- test/test_error_lifecycle.rb | 18 +++++------------- test/test_timeout.rb | 4 ++-- 4 files changed, 42 insertions(+), 19 deletions(-) diff --git a/lib/timeout.rb b/lib/timeout.rb index 39c47cf..5baeb90 100644 --- a/lib/timeout.rb +++ b/lib/timeout.rb @@ -67,6 +67,13 @@ def initialize(thread, timeout, exception_class, message) @done = false # protected by @mutex end + def in_runner_thread + @runner_thread = Thread.new do + yield + end + @runner_thread.value + end + def done? @mutex.synchronize do @done @@ -80,12 +87,26 @@ def expired?(now) def interrupt @mutex.synchronize do unless @done - @thread.raise @exception_class, @message + return unless @runner_thread + @runner_thread.raise @exception_class, @message @done = true + next unless @runner_thread.alive? + @runner_thread.join(2) + # @runner_thread.stop ?? + @thread.raise @exception_class, @message end end end + # def interrupt_main_thread + # @mutex.synchronize do + # unless @done + # @thread.raise @exception_class, @message + # @done = true + # end + # end + # end + def finished @mutex.synchronize do @done = true @@ -112,7 +133,13 @@ def self.create_timeout_thread end requests.each do |req| - req.interrupt if req.expired?(now) + if req.expired?(now) + req.interrupt + + # binding.irb + # puts "\n\ninterrupting main thread\n\n" + # req.interrupt_main_thread # sometimes we don't get here, by design + end end requests.reject!(&:done?) end @@ -177,13 +204,17 @@ def timeout(sec, klass = nil, message = nil, &block) #:yield: +sec+ Timeout.ensure_timeout_thread_created perform = Proc.new do |exc| + # runner_thread = Thread.new request = Request.new(Thread.current, sec, exc, message) QUEUE_MUTEX.synchronize do QUEUE << request CONDVAR.signal end begin - return yield(sec) + request.in_runner_thread do + # Thread.new do + yield(sec) + end ensure request.finished end diff --git a/test/error_lifecycle.rb b/test/error_lifecycle.rb index db89cd5..a33a3f8 100644 --- a/test/error_lifecycle.rb +++ b/test/error_lifecycle.rb @@ -18,7 +18,7 @@ def subject(error_to_raise, error_to_rescue) @outer_ensure_has_time_to_finish = false begin - Timeout.timeout(0.001, error_to_raise) do + Timeout.timeout(0.1, error_to_raise) do @inner_attempted = true nil while true rescue error_to_rescue diff --git a/test/test_error_lifecycle.rb b/test/test_error_lifecycle.rb index c20670f..24bb4a8 100644 --- a/test/test_error_lifecycle.rb +++ b/test/test_error_lifecycle.rb @@ -39,9 +39,7 @@ def test_2 core_assertions(s) assert s.inner_rescue # true in 1.9, false in gem 0.2.0, true in gem 0.4.0 - - # UNDESIRED? - assert !s.outer_rescue # false in 1.9 stdlib, true in gem 0.2.0, false in gem 0.4.0 + assert s.outer_rescue # false in 1.9 stdlib, true in gem 0.2.0, false in gem 0.4.0 end # when an exception to raise is StandardError and the inner code does not catch Exception @@ -51,21 +49,17 @@ def test_3 core_assertions(s) assert s.inner_rescue - - # UNDESIRED? - assert !s.outer_rescue + assert s.outer_rescue end - # when an exception to raise is StandardError and the inner code does catch Exception + # # when an exception to raise is StandardError and the inner code does catch Exception def test_4 s = ErrorLifeCycleTester.new s.subject(MyStandardError, Exception) core_assertions(s) assert s.inner_rescue - - # UNDESIRED? - assert !s.outer_rescue + assert s.outer_rescue end # when an exception to raise is Exception and the inner code does not catch Exception @@ -85,9 +79,7 @@ def test_6 core_assertions(s) assert s.inner_rescue - - # UNDESIRED? - assert !s.outer_rescue + assert s.outer_rescue end end diff --git a/test/test_timeout.rb b/test/test_timeout.rb index 236883a..f4624d8 100644 --- a/test/test_timeout.rb +++ b/test/test_timeout.rb @@ -91,7 +91,7 @@ def test_raises_exception_internally def test_rescue_exit exc = Class.new(RuntimeError) e = nil - assert_nothing_raised(exc) do + assert_raise_with_message(exc, 'execution expired') do Timeout.timeout 0.01, exc do begin sleep 3 @@ -99,7 +99,7 @@ def test_rescue_exit end end end - assert_raise_with_message(exc, 'execution expired') {raise e if e} + # assert_raise_with_message(exc, 'execution expired') {raise e if e} # pretty sure irrelevant now that above behavior is reversed end def test_custom_exception From 8fec9eddb23d470355d48d6148f535ed7944dc47 Mon Sep 17 00:00:00 2001 From: John Bachir Date: Sat, 1 Jul 2023 14:31:19 -0400 Subject: [PATCH 16/27] wip --- lib/timeout.rb | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) diff --git a/lib/timeout.rb b/lib/timeout.rb index 5baeb90..a1c5719 100644 --- a/lib/timeout.rb +++ b/lib/timeout.rb @@ -92,21 +92,11 @@ def interrupt @done = true next unless @runner_thread.alive? @runner_thread.join(2) - # @runner_thread.stop ?? @thread.raise @exception_class, @message end end end - # def interrupt_main_thread - # @mutex.synchronize do - # unless @done - # @thread.raise @exception_class, @message - # @done = true - # end - # end - # end - def finished @mutex.synchronize do @done = true @@ -133,13 +123,7 @@ def self.create_timeout_thread end requests.each do |req| - if req.expired?(now) - req.interrupt - - # binding.irb - # puts "\n\ninterrupting main thread\n\n" - # req.interrupt_main_thread # sometimes we don't get here, by design - end + req.interrupt if req.expired?(now) end requests.reject!(&:done?) end From b72f64e37866e13d1bf2882f8d2c0d305875daa0 Mon Sep 17 00:00:00 2001 From: John Bachir Date: Sat, 1 Jul 2023 14:32:07 -0400 Subject: [PATCH 17/27] wip --- test/test_error_lifecycle.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_error_lifecycle.rb b/test/test_error_lifecycle.rb index 24bb4a8..ed5bc8f 100644 --- a/test/test_error_lifecycle.rb +++ b/test/test_error_lifecycle.rb @@ -52,7 +52,7 @@ def test_3 assert s.outer_rescue end - # # when an exception to raise is StandardError and the inner code does catch Exception + # when an exception to raise is StandardError and the inner code does catch Exception def test_4 s = ErrorLifeCycleTester.new s.subject(MyStandardError, Exception) From 5f27763797d542df069df6fb4c2db688692e3fb3 Mon Sep 17 00:00:00 2001 From: John Bachir Date: Sat, 1 Jul 2023 14:32:35 -0400 Subject: [PATCH 18/27] wip --- test/test_timeout.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/test_timeout.rb b/test/test_timeout.rb index f4624d8..5c8abd7 100644 --- a/test/test_timeout.rb +++ b/test/test_timeout.rb @@ -99,7 +99,9 @@ def test_rescue_exit end end end - # assert_raise_with_message(exc, 'execution expired') {raise e if e} # pretty sure irrelevant now that above behavior is reversed + + # pretty sure irrelevant now that above behavior is reversed + # assert_raise_with_message(exc, 'execution expired') {raise e if e} end def test_custom_exception From 1203108a50d67f63d84c10e83a890d147fa511b1 Mon Sep 17 00:00:00 2001 From: John Bachir Date: Sat, 1 Jul 2023 14:32:52 -0400 Subject: [PATCH 19/27] wip --- test/test_timeout.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/test/test_timeout.rb b/test/test_timeout.rb index 5c8abd7..3006f59 100644 --- a/test/test_timeout.rb +++ b/test/test_timeout.rb @@ -99,7 +99,6 @@ def test_rescue_exit end end end - # pretty sure irrelevant now that above behavior is reversed # assert_raise_with_message(exc, 'execution expired') {raise e if e} end From b286a7f0efd799b9a830fa0e9058d293c9213cef Mon Sep 17 00:00:00 2001 From: John Bachir Date: Sat, 1 Jul 2023 14:34:26 -0400 Subject: [PATCH 20/27] wip --- test/error_lifecycle.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/error_lifecycle.rb b/test/error_lifecycle.rb index a33a3f8..db89cd5 100644 --- a/test/error_lifecycle.rb +++ b/test/error_lifecycle.rb @@ -18,7 +18,7 @@ def subject(error_to_raise, error_to_rescue) @outer_ensure_has_time_to_finish = false begin - Timeout.timeout(0.1, error_to_raise) do + Timeout.timeout(0.001, error_to_raise) do @inner_attempted = true nil while true rescue error_to_rescue From f12a2859071ada4dce3ad8616168282ceafa1306 Mon Sep 17 00:00:00 2001 From: John Bachir Date: Sat, 1 Jul 2023 14:38:09 -0400 Subject: [PATCH 21/27] wip --- lib/timeout.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/timeout.rb b/lib/timeout.rb index a1c5719..6e256fc 100644 --- a/lib/timeout.rb +++ b/lib/timeout.rb @@ -91,7 +91,7 @@ def interrupt @runner_thread.raise @exception_class, @message @done = true next unless @runner_thread.alive? - @runner_thread.join(2) + @runner_thread.join(@deadline) # time allowed for inner ensure block @thread.raise @exception_class, @message end end From 343f2c85944c7f6cc0686347d3f0cb9f91a7103e Mon Sep 17 00:00:00 2001 From: John Bachir Date: Sat, 1 Jul 2023 14:42:40 -0400 Subject: [PATCH 22/27] wip --- lib/timeout.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/timeout.rb b/lib/timeout.rb index 6e256fc..ee26452 100644 --- a/lib/timeout.rb +++ b/lib/timeout.rb @@ -188,7 +188,6 @@ def timeout(sec, klass = nil, message = nil, &block) #:yield: +sec+ Timeout.ensure_timeout_thread_created perform = Proc.new do |exc| - # runner_thread = Thread.new request = Request.new(Thread.current, sec, exc, message) QUEUE_MUTEX.synchronize do QUEUE << request @@ -196,7 +195,6 @@ def timeout(sec, klass = nil, message = nil, &block) #:yield: +sec+ end begin request.in_runner_thread do - # Thread.new do yield(sec) end ensure From d105defd569d5e4092d6025046e1f5a8ff9cb97b Mon Sep 17 00:00:00 2001 From: John Bachir Date: Sat, 1 Jul 2023 14:45:39 -0400 Subject: [PATCH 23/27] wip --- lib/timeout.rb | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/timeout.rb b/lib/timeout.rb index ee26452..2e04dda 100644 --- a/lib/timeout.rb +++ b/lib/timeout.rb @@ -91,7 +91,12 @@ def interrupt @runner_thread.raise @exception_class, @message @done = true next unless @runner_thread.alive? - @runner_thread.join(@deadline) # time allowed for inner ensure block + + # value given to join is time allowed for inner ensure block. + # choosing to re-use @deadline is semi arbitrary. + # implication is total code runtime within Timeout.timeout block is (2 x @timeout) + # (with timeout gem 0.4.0, it is infinite) + @runner_thread.join(@deadline) @thread.raise @exception_class, @message end end From 58f3d74393ed719da43781083d97ee8c31fbadab Mon Sep 17 00:00:00 2001 From: John Bachir Date: Sat, 1 Jul 2023 14:46:56 -0400 Subject: [PATCH 24/27] wip --- lib/timeout.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/timeout.rb b/lib/timeout.rb index 2e04dda..b5ffd94 100644 --- a/lib/timeout.rb +++ b/lib/timeout.rb @@ -58,6 +58,7 @@ class Request attr_reader :deadline def initialize(thread, timeout, exception_class, message) + @timeout = timeout @thread = thread @deadline = GET_TIME.call(Process::CLOCK_MONOTONIC) + timeout @exception_class = exception_class @@ -93,10 +94,10 @@ def interrupt next unless @runner_thread.alive? # value given to join is time allowed for inner ensure block. - # choosing to re-use @deadline is semi arbitrary. + # choosing to re-use @timeout is semi arbitrary. # implication is total code runtime within Timeout.timeout block is (2 x @timeout) # (with timeout gem 0.4.0, it is infinite) - @runner_thread.join(@deadline) + @runner_thread.join(@timeout) @thread.raise @exception_class, @message end end From 59540aaf4bb0a484b451fa6590a80c3fce852529 Mon Sep 17 00:00:00 2001 From: John Bachir Date: Sat, 1 Jul 2023 14:59:02 -0400 Subject: [PATCH 25/27] wip --- lib/timeout.rb | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/lib/timeout.rb b/lib/timeout.rb index b5ffd94..b7c9833 100644 --- a/lib/timeout.rb +++ b/lib/timeout.rb @@ -69,6 +69,9 @@ def initialize(thread, timeout, exception_class, message) end def in_runner_thread + + # do we want to be able to pass down handle_interrupt values from parent thread? + # or does that already happen by default? That test does pass. @runner_thread = Thread.new do yield end @@ -94,10 +97,14 @@ def interrupt next unless @runner_thread.alive? # value given to join is time allowed for inner ensure block. - # choosing to re-use @timeout is semi arbitrary. - # implication is total code runtime within Timeout.timeout block is (2 x @timeout) - # (with timeout gem 0.4.0, it is infinite) - @runner_thread.join(@timeout) + # implication is total code runtime within Timeout.timeout block is (@timeout + whatever is given here) + # with timeout gem 0.4.0, it is infinite + # + # a sensible value to give here would be @timeout, capping total code execution time at 2x@timeout. + # because of how test_handle_interrupt is currently done, doing so makes it fail + # i don't know if the big delta between the timeout numbers in that test are crucial to its behavior check + # hardcoding 5 in here for now + @runner_thread.join(5) # replace with @timeout? @thread.raise @exception_class, @message end end From c87e90e16f34469c771dbfd64a21c6afcf83f810 Mon Sep 17 00:00:00 2001 From: John Bachir Date: Sat, 1 Jul 2023 14:59:43 -0400 Subject: [PATCH 26/27] wip --- lib/timeout.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/timeout.rb b/lib/timeout.rb index b7c9833..ee4392e 100644 --- a/lib/timeout.rb +++ b/lib/timeout.rb @@ -71,7 +71,7 @@ def initialize(thread, timeout, exception_class, message) def in_runner_thread # do we want to be able to pass down handle_interrupt values from parent thread? - # or does that already happen by default? That test does pass. + # or does that already happen by default? test_handle_interrupt does pass. @runner_thread = Thread.new do yield end From 255a3d792d3456ae93729c07a1fd4f8bed458785 Mon Sep 17 00:00:00 2001 From: John Bachir Date: Sat, 1 Jul 2023 15:28:08 -0400 Subject: [PATCH 27/27] experiment --- lib/timeout.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/timeout.rb b/lib/timeout.rb index ee4392e..0925630 100644 --- a/lib/timeout.rb +++ b/lib/timeout.rb @@ -94,7 +94,6 @@ def interrupt return unless @runner_thread @runner_thread.raise @exception_class, @message @done = true - next unless @runner_thread.alive? # value given to join is time allowed for inner ensure block. # implication is total code runtime within Timeout.timeout block is (@timeout + whatever is given here) @@ -105,6 +104,7 @@ def interrupt # i don't know if the big delta between the timeout numbers in that test are crucial to its behavior check # hardcoding 5 in here for now @runner_thread.join(5) # replace with @timeout? + next unless @runner_thread.alive? @thread.raise @exception_class, @message end end