More timeout options for sspec tests. Working every_ events. (#521)

lib/scarpe/cats_cradle.rb

@@ -20,14 +20,17 @@ def initialize
 
       @waiting_fibers = []
       @event_promises = {}
+      @shutdown = false
 
       @manager_fiber = Fiber.new do
+        Fiber[:catscradle] = true
+
         loop do
           # A fiber can run briefly and then exit. It can run and then block on an API call.
           # These fibers return promises to indicate to CatsCradle when they can run again.
           # A fiber that is no longer #alive? is assumed to be successfully finished.
           @waiting_fibers.each do |fiber_data|
-            next unless fiber_data[:promise].fulfilled?
+            next if !fiber_data[:promise].fulfilled? || !fiber_data[:fiber].alive? || @shutdown
 
             @log.debug("Resuming fiber with value #{fiber_data[:promise].returned_value.inspect}")
             result = fiber_data[:fiber].transfer fiber_data[:promise].returned_value
@@ -54,6 +57,17 @@ def initialize
       end
     end
 
+    private
+
+    def cc_fiber(&block)
+      Fiber.new do
+        Fiber[:catscradle] = true
+        CCInstance.instance.instance_eval(&block)
+      end
+    end
+
+    public
+
     # If we add "every" events, that's likely to complicate timing and event_promise handling.
     EVENT_TYPES = [:init, :next_heartbeat, :next_redraw, :every_heartbeat, :every_redraw]
 
@@ -77,9 +91,16 @@ def event_init
           p = @event_promises.delete(:every_heartbeat)
           p&.fulfilled!
 
+          # Reschedule on_every_heartbeat fibers for next heartbeat, too.
+          # This fiber won't be called again by a heartbeat, though it may
+          # continue if it waits on another promise.
+          @waiting_fibers.select { |f| f[:on_event] == :every_heartbeat }.each do |f|
+            on_event(:every_heartbeat, &f[:block])
+          end
+
           # Give every ready fiber a chance to run once.
-          @manager_fiber.resume
-        end
+          @manager_fiber.resume unless @shutdown
+        end unless @shutdown
       end
 
       @control_interface.on_event(:every_redraw) do
@@ -90,14 +111,19 @@ def event_init
           p = @event_promises.delete(:every_redraw)
           p&.fulfilled!
 
+          # Reschedule on_every_redraw fibers for next redraw, too.
+          @waiting_fibers.select { |f| f[:on_event] == :every_redraw }.each do |f|
+            on_event(:every_redraw, &f[:block])
+          end
+
           # Give every ready fiber a chance to run once.
-          @manager_fiber.resume
-        end
+          @manager_fiber.resume unless @shutdown
+        end unless @shutdown
       end
     end
 
     def fiber_start
-      @manager_fiber.resume
+      @manager_fiber.resume unless @shutdown
     end
 
     def event_promise(event)
@@ -106,20 +132,17 @@ def event_promise(event)
 
     def on_event(event, &block)
       raise Scarpe::UnknownEventTypeError, "Unknown event type: #{event.inspect}!" unless EVENT_TYPES.include?(event)
+      return if @shutdown
 
-      f = Fiber.new do
-        CCInstance.instance.instance_eval(&block)
-      end
-      @waiting_fibers << { promise: event_promise(event), fiber: f }
+      @waiting_fibers << { promise: event_promise(event), fiber: cc_fiber(&block), on_event: event, block: }
     end
 
     def active_fiber(&block)
-      f = Fiber.new do
-        CCInstance.instance.instance_eval(&block)
-      end
+      return if @shutdown
+
       p = ::Scarpe::Promise.new
       p.fulfilled!
-      @waiting_fibers << { promise: p, fiber: f }
+      @waiting_fibers << { promise: p, fiber: cc_fiber(&block), on_event: nil, block: }
     end
 
     def wait(promise)
@@ -156,6 +179,11 @@ def query_js_promise(js_code, timeout: 1.0)
     end
 
     def shut_down_shoes_code
+      if @shutdown
+        exit 0
+      end
+
+      @shutdown = true
       ::Shoes::DisplayService.dispatch_event("destroy", nil)
     end
   end

lib/scarpe/shoes_spec.rb

@@ -26,17 +26,26 @@ def self.run_shoes_spec_test_code(code, class_name: nil, test_name: nil)
     class_name ||= ENV["SHOES_MINITEST_CLASS_NAME"] || "TestShoesSpecCode"
     test_name ||= ENV["SHOES_MINITEST_METHOD_NAME"] || "test_shoes_spec"
 
+    Scarpe::CCInstance.include Scarpe::ShoesSpecTest
+
     Scarpe::CCInstance.instance.instance_eval do
       event_init
 
+      t_timeout = ENV["SCARPE_SSPEC_TIMEOUT"] || "30"
+      timeout(t_timeout.to_f) unless t_timeout.downcase == "none"
+
       on_event(:next_heartbeat) do
         Minitest.run ARGV
 
-        shut_down_shoes_code
+        wait_after = ENV["SCARPE_SSPEC_TIMEOUT_WAIT_AFTER_TEST"]
+        if !(wait_after && wait_after.downcase != "n" && wait_after.downcase != "no")
+          shut_down_shoes_code
+        end
       end
     end
 
-    test_class = Class.new(Scarpe::ShoesSpecTest)
+    test_class = Class.new(Minitest::Test)
+    test_class.include Scarpe::ShoesSpecTest
     Object.const_set(Scarpe::Components::StringHelpers.camelize(class_name), test_class)
     test_name = "test_" + test_name unless test_name.start_with?("test_")
     test_class.define_method(test_name) do
@@ -91,7 +100,7 @@ def respond_to_missing?(method_name, include_private = false)
 
 # When running ShoesSpec tests, we create a parent class for all of them
 # with the appropriate convenience methods and accessors.
-class Scarpe::ShoesSpecTest < Minitest::Test
+module Scarpe::ShoesSpecTest
   include Scarpe::Test::HTMLAssertions
 
   Shoes::Drawable.drawable_classes.each do |drawable_class|
@@ -126,30 +135,25 @@ def dom_html
     end
   end
 
-  # This isn't working. Neither is calling die_after. Are the other fibers not
-  # running or something like that? Should run a test from the command line
-  # and see what's happening... Or check logfiles?
-  def timeout(t_timeout = 5.0, exit_code: -1)
+  # A timeout won't cause an error by itself. If you want an error, make sure
+  # to check for a minimum number of assertions or otherwise look for progress.
+  def timeout(t_timeout = 5.0)
     catscradle_dsl do
       t0 = Time.now
       on_event(:every_heartbeat) do
         if Time.now - t0 >= t_timeout
-          if exit_code == 0
-            @log.info "Timed out after #{t_timeout} seconds!"
-          else
-            @log.error "Timed out after #{t_timeout} seconds!"
-          end
-          exit exit_code
+          @log.info "Timed out after #{t_timeout} seconds!"
+          shut_down_shoes_code
         end
       end
     end
   end
 
-  def exit_on_first_heartbeat(exit_code: 0)
+  def exit_on_first_heartbeat
     catscradle_dsl do
       on_event(:next_heartbeat) do
         @log.info "Exiting on first heartbeat (exit code #{exit_code})"
-        exit exit_code
+        exit 0
       end
     end
   end

scarpe-components/lib/scarpe/components/minitest_result.rb

@@ -63,13 +63,21 @@ def one_word_result
     "success"
   end
 
+  def result_and_message
+    return ["error", error_message] if self.error?
+    return ["fail", fail_message] if self.fail?
+    return ["skip", skip_message] if self.skip?
+    ["success", "OK"]
+  end
+
   def check(expect_result: :success, min_asserts: nil, max_asserts: nil)
     unless [:error, :fail, :skip, :success].include?(expect_result)
       raise Scarpe::InternalError, "Expected test result should be one of [:success, :fail, :error, :skip]!"
     end
 
-    if expect_result.to_s != one_word_result
-      return [false, "Expected #{expect_result} but got #{one_word_result}!"]
+    res, msg = result_and_message
+    if expect_result.to_s != res
+      return [false, "Expected #{expect_result} but got #{res}: #{msg}!"]
     end
 
     if min_asserts && @assertions < min_asserts

test/test_helper.rb

@@ -47,6 +47,8 @@ def run_scarpe_sspec(
     expect_assertions_min: nil,
     expect_assertions_max: nil,
     expect_result: :success,
+    timeout: 10.0, # or :none for no timeout
+    wait_after_test: false,
     display_service: "wv_local",
     html_renderer: "calzini"
   )
@@ -61,6 +63,8 @@ def run_scarpe_sspec(
         "SCARPE_DISPLAY_SERVICE=#{display_service} " +
         "SCARPE_HTML_RENDERER=#{html_renderer} " +
         "SCARPE_LOG_CONFIG=\"#{scarpe_log_config}\" " +
+        "SCARPE_SSPEC_TIMEOUT=\"#{timeout}\" " +
+        "#{wait_after_test ? "SCARPE_SSPEC_TIMEOUT_WAIT_AFTER_TEST=Y" : ""} " +
         "SHOES_MINITEST_EXPORT_FILE=\"#{test_output}\" " +
         "SHOES_MINITEST_CLASS_NAME=\"#{test_class_name}\" " +
         "SHOES_MINITEST_METHOD_NAME=\"#{test_method_name}\" " +
@@ -72,7 +76,7 @@ def run_scarpe_sspec(
         if process_success
           assert false, "Expected sspec test process to return success and it failed! Exit code: #{$?.exitstatus}"
         else
-          assert false, "Expected sspec test process to return failure and it succeeded!"
+          assert false, "Expected sspec test process to return failure and it succeeded! Exit code: #{$?.exitstatus}"
         end
       end
     end

test/test_sspec_infrastructure.rb

@@ -18,6 +18,55 @@ def test_simple_assertion_success
     SSPEC
   end
 
+  def test_empty_assertions
+    run_scarpe_sspec_code(<<~'SSPEC')
+      ---
+      ----------- app code
+      Shoes.app do
+      end
+      ----------- test code
+      # Without at least a comment, the file parser doesn't catch that this section is here.
+    SSPEC
+  end
+
+  # Here's a weird thing: we want to detect the test failure somehow. And right now we
+  # can't usefully tell the timeout to cause the *process* to fail just from the timeout.
+  # We'll still notice test failures or not hitting enough assertions.
+  def test_timeout_no_fail
+    run_scarpe_sspec_code(<<~'SSPEC', timeout: 1.0, wait_after_test: true, expect_assertions_min: 1)
+      ---
+      ----------- app code
+      Shoes.app do
+      end
+      ----------- test code
+      assert_equal true, true
+    SSPEC
+  end
+
+  def test_timeout_test_fail
+    run_scarpe_sspec_code(<<~'SSPEC', timeout: 1.0, wait_after_test: true, expect_result: :fail)
+      ---
+      ----------- app code
+      Shoes.app do
+      end
+      ----------- test code
+      assert_equal false, true
+    SSPEC
+  end
+
+  # Specify ":none" for timeout. In this case it will still finish promptly -- just checking
+  # that the setting works.
+  def test_no_timeout
+    run_scarpe_sspec_code(<<~'SSPEC', timeout: :none)
+      ---
+      ----------- app code
+      Shoes.app do
+      end
+      ----------- test code
+      assert_equal true, true
+    SSPEC
+  end
+
   def test_exception
     run_scarpe_sspec_code(<<~'SSPEC', expect_result: :error)
       ---