Fix #5162 - Capture exit from Ruby

python/engine/PythonEngine.cpp

@@ -253,34 +253,65 @@ void* PythonEngine::getAs_impl(ScriptObject& obj, const std::type_info& ti) {
 
   void* return_value = nullptr;
 
-  if (ti == typeid(std::string*)) {
-    // TODO: this sucks, and probably needs a PyDecref or two
-    Py_ssize_t size = 0;
-    char const* pc = PyUnicode_AsUTF8AndSize(val.obj_, &size);
-    if (pc) {
-      return_value = new std::string(pc, size);
-    } else {
-      throw std::runtime_error("Unable to convert to std::string in SWIG Python");
-    }
-  } else {
-    const auto& type_name = getRegisteredTypeName(ti);
+  const auto& type_name = getRegisteredTypeName(ti);
 
-    auto* type = SWIG_Python_TypeQuery(type_name.c_str());
+  auto* type = SWIG_Python_TypeQuery(type_name.c_str());
 
-    if (!type) {
-      throw std::runtime_error("Unable to find type in SWIG");
-    }
+  if (!type) {
+    throw std::runtime_error("Unable to find type in SWIG");
+  }
 
-    const auto result = SWIG_Python_ConvertPtr(val.obj_, &return_value, type, 0);
+  const auto result = SWIG_Python_ConvertPtr(val.obj_, &return_value, type, 0);
 
-    if (!SWIG_IsOK(result)) {
-      throw std::runtime_error(fmt::format("Error getting object from SWIG/Python of supposed type {}, {}\n", ti.name(), type_name.c_str()));
-    }
+  if (!SWIG_IsOK(result)) {
+    throw std::runtime_error(fmt::format("Error getting object from SWIG/Python of supposed type {}, {}\n", ti.name(), type_name.c_str()));
   }
 
   return return_value;
 }
 
+bool PythonEngine::getAs_impl_bool(ScriptObject& obj) {
+  auto val = std::any_cast<PythonObject>(obj.object);
+  if (!PyBool_Check(val.obj_)) {
+    throw std::runtime_error("PyObject is not a bool");
+  }
+  return static_cast<bool>(PyObject_IsTrue(val.obj_));
+}
+
+int PythonEngine::getAs_impl_int(ScriptObject& obj) {
+  auto val = std::any_cast<PythonObject>(obj.object);
+  if (!PyLong_Check(val.obj_)) {
+    throw std::runtime_error("PyObject is not a PyLong");
+  }
+
+  return static_cast<int>(PyLong_AsLong(val.obj_));
+}
+
+double PythonEngine::getAs_impl_double(ScriptObject& obj) {
+  auto val = std::any_cast<PythonObject>(obj.object);
+  if (!PyFloat_Check(val.obj_)) {
+    throw std::runtime_error("PyObject is not a PyFloat");
+  }
+
+  return PyFloat_AsDouble(val.obj_);
+}
+
+std::string PythonEngine::getAs_impl_string(ScriptObject& obj) {
+  auto val = std::any_cast<PythonObject>(obj.object);
+
+  if (!PyUnicode_Check(val.obj_)) {
+    throw std::runtime_error("PyObject is not a String");
+  }
+
+  Py_ssize_t size = 0;
+  char const* pc = PyUnicode_AsUTF8AndSize(val.obj_, &size);  // No decref needed
+  if (!pc) {
+    throw std::runtime_error("Unable to convert to std::string in SWIG Python");
+  }
+
+  return std::string{pc, static_cast<size_t>(size)};
+}
+
 std::string PythonEngine::inferMeasureClassName(const openstudio::path& measureScriptPath) {
 
   auto inferClassNameCmd = fmt::format(R"python(
@@ -300,7 +331,7 @@ measure_name, measure_typeinfo = class_members[0]
   try {
     exec(inferClassNameCmd);
     ScriptObject measureClassNameObject = eval("measure_name");
-    className = *getAs<std::string*>(measureClassNameObject);
+    className = getAs<std::string>(measureClassNameObject);
   } catch (const std::runtime_error& e) {
     auto msg = fmt::format("Failed to infer measure name from {}: {}", measureScriptPath.generic_string(), e.what());
     fmt::print(stderr, "{}\n", msg);

python/engine/PythonEngine.hpp

@@ -40,6 +40,12 @@ class PythonEngine final : public ScriptEngine
 
  protected:
   void* getAs_impl(ScriptObject& obj, const std::type_info&) override;
+
+  bool getAs_impl_bool(ScriptObject& obj) override;
+  int getAs_impl_int(ScriptObject& obj) override;
+  double getAs_impl_double(ScriptObject& obj) override;
+  std::string getAs_impl_string(ScriptObject& obj) override;
+
   void importOpenStudio();
   void pyimport(const std::string& importName, const std::string& includePath);
 

ruby/engine/RubyEngine.cpp

@@ -75,7 +75,13 @@ void RubyEngine::setupEmbeddedGems(const std::vector<openstudio::path>& includeD
 }
 
 RubyEngine::~RubyEngine() {
-  ruby_finalize();
+  // ruby_cleanup calls ruby_finalize
+  int ex = ruby_cleanup(0);
+  if (ex != 0) {
+    fmt::print("RubyEngine return code was {}\n", ex);
+    exit(ex);
+  }
+  //ruby_finalize();
 }
 
 ScriptObject RubyEngine::eval(std::string_view sv) {
@@ -95,30 +101,56 @@ void* RubyEngine::getAs_impl(ScriptObject& obj, const std::type_info& ti) {
 
   void* return_value = nullptr;
 
-  // TODO: this sucks, and probably needs memory management
-  if (ti == typeid(std::string*)) {
-    char* cstr = StringValuePtr(val);
-    size_t size = RSTRING_LEN(val);  // + 1;  From trial and eror, if I had + 1 I get a string with one two many size
-    return_value = new std::string(cstr, size);
+  const auto& type_name = getRegisteredTypeName(ti);
 
-    // std::string s = rb_string_value_cstr(&val);
-  } else {
-    const auto& type_name = getRegisteredTypeName(ti);
-    auto* type = SWIG_TypeQuery(type_name.c_str());
+  auto* type = SWIG_TypeQuery(type_name.c_str());
 
-    if (!type) {
-      throw std::runtime_error("Unable to find type in SWIG");
-    }
+  if (!type) {
+    throw std::runtime_error("Unable to find type in SWIG");
+  }
 
-    const auto result = SWIG_ConvertPtr(val, &return_value, type, 0);
+  const auto result = SWIG_ConvertPtr(val, &return_value, type, 0);
 
-    if (!SWIG_IsOK(result)) {
-      throw std::runtime_error("Error getting object from SWIG/Ruby");
-    }
+  if (!SWIG_IsOK(result)) {
+    throw std::runtime_error("Error getting object from SWIG/Ruby");
   }
   return return_value;
 }
 
+bool RubyEngine::getAs_impl_bool(ScriptObject& obj) {
+  auto val = std::any_cast<VALUE>(obj.object);
+  return RB_TEST(val);
+}
+
+int RubyEngine::getAs_impl_int(ScriptObject& obj) {
+  auto val = std::any_cast<VALUE>(obj.object);
+  if (!RB_FIXNUM_P(val)) {
+    throw std::runtime_error("VALUE is not a FIXNUM");
+  }
+
+  return RB_NUM2INT(val);
+}
+double RubyEngine::getAs_impl_double(ScriptObject& obj) {
+  auto val = std::any_cast<VALUE>(obj.object);
+  if (!RB_FLOAT_TYPE_P(val)) {
+    throw std::runtime_error("VALUE is not a FLOAT");
+  }
+
+  return RB_NUM2INT(val);
+}
+
+std::string RubyEngine::getAs_impl_string(ScriptObject& obj) {
+  auto val = std::any_cast<VALUE>(obj.object);
+
+  if (!RB_TYPE_P(val, RUBY_T_STRING)) {
+    throw std::runtime_error("VALUE is not a String");
+  }
+
+  char* cstr = StringValuePtr(val);
+  size_t size = RSTRING_LEN(val);  // + 1;  From trial and eror, if I had + 1 I get a string with one two many size
+  return std::string{cstr, size};
+}
+
 std::string RubyEngine::inferMeasureClassName(const openstudio::path& measureScriptPath) {
 
   auto inferClassNameCmd = fmt::format(R"ruby(
@@ -161,7 +193,7 @@ ObjectSpace.garbage_collect
     ScriptObject measureClassNameObject = eval("$measure_name");
     // measureClassNameObject = rubyEngine->eval(fmt::format("{}.new()", className));
     // ScriptObject measureClassNameObject = rubyEngine->eval(inferClassName);
-    className = *getAs<std::string*>(measureClassNameObject);
+    className = getAs<std::string>(measureClassNameObject);
   } catch (const RubyException& e) {
     auto msg = fmt::format("Failed to infer measure name from {}: {}\nlocation={}", measureScriptPath.generic_string(), e.what(), e.location());
     fmt::print(stderr, "{}\n", msg);

ruby/engine/RubyEngine.hpp

@@ -44,6 +44,11 @@ class RubyEngine final : public ScriptEngine
   // so the above template function can provide it back to the caller.
   void* getAs_impl(ScriptObject& obj, const std::type_info&) override;
 
+  bool getAs_impl_bool(ScriptObject& obj) override;
+  int getAs_impl_int(ScriptObject& obj) override;
+  double getAs_impl_double(ScriptObject& obj) override;
+  std::string getAs_impl_string(ScriptObject& obj) override;
+
   void initRubyEngine();
   std::vector<std::string> includePaths;
   RubyInterpreter rubyInterpreter{includePaths};

ruby/test/CMakeLists.txt

@@ -66,14 +66,16 @@ if(BUILD_TESTING)
         string(REGEX MATCH "/?([A-Za-z_0-9 ]+)\\.rb" FILE_NAME ${f})
         string(REGEX REPLACE "/?([A-Za-z_0-9 ]+)\\.rb" "\\1" FILE_NAME ${FILE_NAME})
 
+        set(CTEST_NAME "RubyTest-${FILE_NAME}-${TEST_NAME}")
+
         # Call with Ruby itself
-        add_test(NAME "RubyTest-${FILE_NAME}-${TEST_NAME}"
+        add_test(NAME "${CTEST_NAME}"
           COMMAND "${CMAKE_COMMAND}" -E chdir "${CMAKE_CURRENT_BINARY_DIR}"
           "${SYSTEM_RUBY_EXECUTABLE}" "-I" "$<TARGET_FILE_DIR:openstudio_rb>" "${f}" "--name=test_${TEST_NAME}"
         )
-
-        set_tests_properties("RubyTest-${FILE_NAME}-${TEST_NAME}" PROPERTIES TIMEOUT 660 )
+        set_tests_properties("${CTEST_NAME}" PROPERTIES TIMEOUT 660 )
       endforeach()
+
     endforeach()
 
   endif()

src/cli/CMakeLists.txt

@@ -476,6 +476,17 @@ if(BUILD_TESTING)
     message(AUTHOR_WARNING "Cannot run the python numpy test, as numpy isn't installed on your system python")
   endif()
 
+  if (Python_EXECUTABLE)
+    add_test(NAME OpenStudioCLI.minitest_fail
+      COMMAND ${Python_EXECUTABLE} "${CMAKE_CURRENT_SOURCE_DIR}/test/run_minitest_fail.py" $<TARGET_FILE:openstudio>
+    )
+    if (SYSTEM_RUBY_EXECUTABLE)
+      add_test(NAME RubyTest.minitest_fail
+        COMMAND ${Python_EXECUTABLE} "${CMAKE_CURRENT_SOURCE_DIR}/test/run_minitest_fail.py" "${SYSTEM_RUBY_EXECUTABLE}"
+      )
+    endif()
+  endif()
+
   file(GLOB RUBY_TEST_SRC
     # find all CLI test
     "test/test*.rb"
@@ -498,13 +509,16 @@ if(BUILD_TESTING)
       string(REGEX MATCH "/?([A-Za-z_0-9 ]+)\\.rb" FILE_NAME ${f})
       string(REGEX REPLACE "/?([A-Za-z_0-9 ]+)\\.rb" "\\1" FILE_NAME ${FILE_NAME})
 
+      set(CTEST_NAME "CLITest-${FILE_NAME}-${TEST_NAME}")
 
-          add_test(NAME "CLITest-${FILE_NAME}-${TEST_NAME}"
-            COMMAND "${CMAKE_COMMAND}" -E chdir "${CMAKE_CURRENT_BINARY_DIR}"
-            "$<TARGET_FILE:openstudio>" "${f}" "--name=test_${TEST_NAME}"
-          )
+      add_test(NAME "${CTEST_NAME}"
+        COMMAND "${CMAKE_COMMAND}" -E chdir "${CMAKE_CURRENT_BINARY_DIR}"
+        "$<TARGET_FILE:openstudio>" "${f}" "--name=test_${TEST_NAME}"
+        )
 
-        set_tests_properties("CLITest-${FILE_NAME}-${TEST_NAME}" PROPERTIES TIMEOUT 660 )
+      set_tests_properties("${CTEST_NAME}" PROPERTIES TIMEOUT 660 )
     endforeach()
+
   endforeach()
+
 endif()

src/cli/UpdateCommand.cpp

@@ -69,19 +69,31 @@ namespace cli {
     cmd += fmt::format(R"(
 begin
   require '{}'
-rescue SystemExit
-  # puts "help was called"
+  0
+rescue SystemExit => e
+  # puts "System Exit: #{{e.status}}"
+  if !e.success?
+    STDERR.puts
+    STDERR.puts "#{{e.class}}: #{{e.message}} with return code #{{e.status}} "
+    STDERR.puts "Backtrace:\n\t" + e.backtrace.join("\n\t")
+  end
+  e.status
 rescue Exception => e
   STDERR.puts
   STDERR.puts "#{{e.class}}: #{{e.message}}"
   STDERR.puts "Backtrace:\n\t" + e.backtrace.join("\n\t")
-  STDERR.flush
   raise
 end
      )",
                        rubyScriptPath.generic_string());
     try {
-      rubyEngine->exec(cmd);
+      auto ret_so = rubyEngine->eval(cmd);
+      auto ret_code = rubyEngine->getAs<int>(ret_so);
+      // If ret_code is already none zero, just exit faster
+      // Otherwise let it be, so that the at_exit(s) can run in particular (for minitest for eg)
+      if (ret_code != 0) {
+        exit(ret_code);
+      }
     } catch (...) {
       // Bail faster though ruby isn't slow like python
       fmt::print(stderr, "Failed to execute '{}'\n", rubyScriptPath.generic_string());

src/cli/main.cpp

@@ -199,8 +199,6 @@ int main(int argc, char* argv[]) {
       rubyEngine->registerType<openstudio::measure::EnergyPlusMeasure*>("openstudio::measure::EnergyPlusMeasure *");
       rubyEngine->registerType<openstudio::measure::ReportingMeasure*>("openstudio::measure::ReportingMeasure *");
       rubyEngine->registerType<openstudio::measure::MeasureInfoBinding*>("openstudio::measure::MeasureInfoBinding *");
-      // rubyEngine->registerType<std::string>("std::string");
-      // rubyEngine->registerType<std::string*>("std::string *");
       rubyEngine->exec("OpenStudio::init_rest_of_openstudio()");
     };
     rubyEngine.registerInitializationFunction(runSetupEmbeddedGems);
@@ -229,7 +227,6 @@ int main(int argc, char* argv[]) {
       pythonEngine->registerType<openstudio::measure::ModelMeasure*>("openstudio::measure::ModelMeasure *");
       pythonEngine->registerType<openstudio::measure::EnergyPlusMeasure*>("openstudio::measure::EnergyPlusMeasure *");
       pythonEngine->registerType<openstudio::measure::ReportingMeasure*>("openstudio::measure::ReportingMeasure *");
-      // pythonEngine->registerType<std::string*>("std::string *");
     };
     pythonEngine.registerInitializationFunction(runSetupPythonPath);
 

src/cli/test/at_exit.rb

@@ -0,0 +1,3 @@
+at_exit {
+  exit 12
+}

src/cli/test/minitest_fail.rb

@@ -0,0 +1,8 @@
+require 'minitest'
+require 'minitest/autorun'
+
+class MyTest < Minitest::Test
+  def test_should_fail
+    assert 0 == 1
+  end
+end

src/cli/test/minitest_ok.rb

@@ -0,0 +1,8 @@
+require 'minitest'
+require 'minitest/autorun'
+
+class MyTest < Minitest::Test
+  def test_should_pass
+    assert 0 == 0
+  end
+end