diff --git a/python/engine/PythonEngine.cpp b/python/engine/PythonEngine.cpp index 95161e7f3bf..fdeb1cdedda 100644 --- a/python/engine/PythonEngine.cpp +++ b/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(obj.object); + if (!PyBool_Check(val.obj_)) { + throw std::runtime_error("PyObject is not a bool"); + } + return static_cast(PyObject_IsTrue(val.obj_)); +} + +int PythonEngine::getAs_impl_int(ScriptObject& obj) { + auto val = std::any_cast(obj.object); + if (!PyLong_Check(val.obj_)) { + throw std::runtime_error("PyObject is not a PyLong"); + } + + return static_cast(PyLong_AsLong(val.obj_)); +} + +double PythonEngine::getAs_impl_double(ScriptObject& obj) { + auto val = std::any_cast(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(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)}; +} + 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(measureClassNameObject); + className = getAs(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); diff --git a/python/engine/PythonEngine.hpp b/python/engine/PythonEngine.hpp index 0252e59586b..f05019e0e9e 100644 --- a/python/engine/PythonEngine.hpp +++ b/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); diff --git a/ruby/engine/RubyEngine.cpp b/ruby/engine/RubyEngine.cpp index 232fa69a6c4..957ec283eac 100644 --- a/ruby/engine/RubyEngine.cpp +++ b/ruby/engine/RubyEngine.cpp @@ -75,7 +75,13 @@ void RubyEngine::setupEmbeddedGems(const std::vector& 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(obj.object); + return RB_TEST(val); +} + +int RubyEngine::getAs_impl_int(ScriptObject& obj) { + auto val = std::any_cast(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(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(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(measureClassNameObject); + className = getAs(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); diff --git a/ruby/engine/RubyEngine.hpp b/ruby/engine/RubyEngine.hpp index a38b4364186..671da266430 100644 --- a/ruby/engine/RubyEngine.hpp +++ b/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 includePaths; RubyInterpreter rubyInterpreter{includePaths}; diff --git a/ruby/test/CMakeLists.txt b/ruby/test/CMakeLists.txt index 62a5c9c66d1..97f8aed5628 100644 --- a/ruby/test/CMakeLists.txt +++ b/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" "$" "${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() diff --git a/src/cli/CMakeLists.txt b/src/cli/CMakeLists.txt index 009fe3b256d..77a30c1b3ff 100644 --- a/src/cli/CMakeLists.txt +++ b/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" $ + ) + 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}" - "$" "${f}" "--name=test_${TEST_NAME}" - ) + add_test(NAME "${CTEST_NAME}" + COMMAND "${CMAKE_COMMAND}" -E chdir "${CMAKE_CURRENT_BINARY_DIR}" + "$" "${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() diff --git a/src/cli/UpdateCommand.cpp b/src/cli/UpdateCommand.cpp index 5e3deea03ee..bd96d463096 100644 --- a/src/cli/UpdateCommand.cpp +++ b/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(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()); diff --git a/src/cli/main.cpp b/src/cli/main.cpp index 920510f5c12..2449aa26689 100644 --- a/src/cli/main.cpp +++ b/src/cli/main.cpp @@ -199,8 +199,6 @@ int main(int argc, char* argv[]) { rubyEngine->registerType("openstudio::measure::EnergyPlusMeasure *"); rubyEngine->registerType("openstudio::measure::ReportingMeasure *"); rubyEngine->registerType("openstudio::measure::MeasureInfoBinding *"); - // rubyEngine->registerType("std::string"); - // rubyEngine->registerType("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 *"); pythonEngine->registerType("openstudio::measure::EnergyPlusMeasure *"); pythonEngine->registerType("openstudio::measure::ReportingMeasure *"); - // pythonEngine->registerType("std::string *"); }; pythonEngine.registerInitializationFunction(runSetupPythonPath); diff --git a/src/cli/test/at_exit.rb b/src/cli/test/at_exit.rb new file mode 100644 index 00000000000..1b4e7cda9b5 --- /dev/null +++ b/src/cli/test/at_exit.rb @@ -0,0 +1,3 @@ +at_exit { + exit 12 +} diff --git a/src/cli/test/minitest_fail.rb b/src/cli/test/minitest_fail.rb new file mode 100644 index 00000000000..e7f509b7920 --- /dev/null +++ b/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 diff --git a/src/cli/test/minitest_ok.rb b/src/cli/test/minitest_ok.rb new file mode 100644 index 00000000000..b621cb7d51e --- /dev/null +++ b/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 diff --git a/src/cli/test/run_minitest_fail.py b/src/cli/test/run_minitest_fail.py new file mode 100644 index 00000000000..bf7ae14c2db --- /dev/null +++ b/src/cli/test/run_minitest_fail.py @@ -0,0 +1,74 @@ +import argparse +import subprocess +from pathlib import Path +from typing import Optional + +THIS_DIR = Path(__file__).parent.absolute() +MINITEST_FAIL_FILE = THIS_DIR / "minitest_fail.rb" +MINITEST_OK_FILE = THIS_DIR / "minitest_ok.rb" +AT_EXIT_FILE = THIS_DIR / "at_exit.rb" +ALL_FILES = [MINITEST_FAIL_FILE, MINITEST_OK_FILE, AT_EXIT_FILE] + + +def run_file( + ruby_or_cli_path: str, + filepath: Path, + expected_return_code: int, + expected_stdout_message: Optional[str], + verbose: bool = True, +): + + command = [str(ruby_or_cli_path), str(filepath)] + + if verbose: + print(f"Running: `{' '.join(command)}`") + + r = subprocess.run( + command, stdout=subprocess.PIPE, stderr=subprocess.PIPE, encoding="utf-8", universal_newlines=True + ) + ret_code = r.returncode + if verbose: + print(f"return code: {ret_code}") + print(f"stdout:\n```\n{r.stdout}\n```") + print(f"stderr:\n```\n{r.stderr}\n```") + + assert ret_code == expected_return_code, f"Expected the return code to be {expected_return_code}, got {ret_code}" + if expected_stdout_message is not None: + assert expected_stdout_message in r.stdout, f"stdout appears to be missing the minitest output.\n\n{r.stdout}" + + +if __name__ == "__main__": + parser = argparse.ArgumentParser(description="Run a logger test.") + parser.add_argument("ruby_or_cli_path", type=str, help="Path to the OS CLI or the System Ruby") + args = parser.parse_args() + + for fp in ALL_FILES: + if not fp.is_file(): + raise ValueError(f"{fp} does not exist") + + run_file( + ruby_or_cli_path=args.ruby_or_cli_path, + filepath=AT_EXIT_FILE, + expected_return_code=12, + expected_stdout_message=None, + ) + + run_file( + ruby_or_cli_path=args.ruby_or_cli_path, + filepath=MINITEST_OK_FILE, + expected_return_code=0, + expected_stdout_message="0 failures", + ) + + run_file( + ruby_or_cli_path=args.ruby_or_cli_path, + filepath=MINITEST_FAIL_FILE, + expected_return_code=1, + expected_stdout_message="1 failures", + ) + + r = subprocess.run( + [str(args.ruby_or_cli_path), "-e", "at exit { exit 12 }"], + stdout=subprocess.PIPE, stderr=subprocess.PIPE, encoding="utf-8", universal_newlines=True + ) + assert r.returncode == 12 diff --git a/src/scriptengine/ScriptEngine.hpp b/src/scriptengine/ScriptEngine.hpp index 256d87786e4..766727ba703 100644 --- a/src/scriptengine/ScriptEngine.hpp +++ b/src/scriptengine/ScriptEngine.hpp @@ -81,11 +81,21 @@ class ScriptEngine template T getAs(ScriptObject& obj) { - void* result = getAs_impl(obj, typeid(T)); - if (result) { - return static_cast(result); + if constexpr (std::is_same_v) { + return getAs_impl_bool(obj); + } else if constexpr (std::is_same_v) { + return getAs_impl_int(obj); + } else if constexpr (std::is_same_v) { + return getAs_impl_double(obj); + } else if constexpr (std::is_same_v) { + return getAs_impl_string(obj); } else { - throw std::bad_cast(); + void* result = getAs_impl(obj, typeid(T)); + if (result) { + return static_cast(result); + } else { + throw std::bad_cast(); + } } } @@ -99,6 +109,11 @@ class ScriptEngine // so the above template function can provide it back to the caller. virtual void* getAs_impl(ScriptObject& obj, const std::type_info&) = 0; + virtual bool getAs_impl_bool(ScriptObject& obj) = 0; + virtual int getAs_impl_int(ScriptObject& obj) = 0; + virtual double getAs_impl_double(ScriptObject& obj) = 0; + virtual std::string getAs_impl_string(ScriptObject& obj) = 0; + const std::string& getRegisteredTypeName(const std::type_info& type) { const auto& found_name = types.find(type);