Unit-testing: tests for Nasal SGCommand API

Test adding/removing/invoking commands, and error handles when
duplicate adding and removing a command name.

src/Scripting/NasalSys.cxx

@@ -821,7 +821,12 @@ static naRef f_addCommand(naContext c, naRef me, int argc, naRef* args)
     if(argc != 2 || !naIsString(args[0]) || !naIsFunc(args[1]))
         naRuntimeError(c, "bad arguments to addcommand()");
 
-    nasalSys->addCommand(args[1], naStr_data(args[0]));
+    const string commandName(naStr_data(args[0]));
+    bool ok = nasalSys->addCommand(args[1], commandName);
+    if (!ok) {
+        naRuntimeError(c, "Failed to add command:%s : likely a duplicate name ", commandName.c_str());
+    }
+
     return naNil();
 }
 
@@ -1423,17 +1428,34 @@ void FGNasalSys::loadPropertyScripts(SGPropertyNode* n)
     loaded->setBoolValue(is_loaded);
 }
 
+#if defined(BUILDING_TESTSUITE)
+
+static string_list global_nasalErrors;
+
+string_list FGNasalSys::getAndClearErrorList()
+{
+    string_list r;
+    global_nasalErrors.swap(r);
+    return r;
+}
+#endif
+
 // Logs a runtime error, with stack trace, to the FlightGear log stream
 void FGNasalSys::logError(naContext context)
 {
     string errorMessage = naGetError(context);
+#if defined(BUILDING_TESTSUITE)
+    global_nasalErrors.push_back(errorMessage);
+#else
     SG_LOG(SG_NASAL, SG_ALERT, "Nasal runtime error: " << errorMessage);
 
     string_list nasalStack;
     logNasalStack(context, nasalStack);
     flightgear::sentryReportNasalError(errorMessage, nasalStack);
+#endif
 }
 
+
 void FGNasalSys::logNasalStack(naContext context, string_list& stack)
 {
     const int stack_depth = naStackDepth(context);
@@ -1770,15 +1792,16 @@ void FGNasalSys::registerToUnload(FGNasalModelData *data)
     _unloadList.push(data);
 }
 
-void FGNasalSys::addCommand(naRef func, const std::string& name)
+bool FGNasalSys::addCommand(naRef func, const std::string& name)
 {
     if (_commands.find(name) != _commands.end()) {
         SG_LOG(SG_NASAL, SG_WARN, "duplicate add of command:" << name);
-        return;
+        return false;
     }
 
     NasalCommand* cmd = new NasalCommand(this, func, name);
     _commands[name] = cmd;
+    return true;
 }
 
 bool FGNasalSys::removeCommand(const std::string& name)

src/Scripting/NasalSys.hxx

@@ -94,7 +94,7 @@ public:
 
     naRef getModule(const char* moduleName);
 
-    void addCommand(naRef func, const std::string& name);
+    bool addCommand(naRef func, const std::string& name);
     bool removeCommand(const std::string& name);
 
     /**
@@ -153,6 +153,12 @@ public:
     simgear::BufferedLogCallback* log() const
     { return _log.get(); }
 
+#if defined(BUILDING_TESTSUITE)
+    /// test-suite only API: retrieve all Nasal runtime errors which
+    /// occurred since we last called this
+    string_list getAndClearErrorList();
+#endif
+
 private:
     void initLogLevelConstants();
 

test_suite/unit_tests/Scripting/testNasalSys.cxx

@@ -22,8 +22,12 @@
 
 #include "test_suite/FGTestApi/testGlobals.hxx"
 
+#include <simgear/structure/commands.hxx>
+
+#include <Main/fg_props.hxx>
 #include <Main/globals.hxx>
 #include <Main/util.hxx>
+
 #include <Scripting/NasalSys.hxx>
 
 #include <Main/FGInterpolator.hxx>
@@ -89,3 +93,60 @@ void NasalSysTests::testStructEquality()
     )");
     CPPUNIT_ASSERT(ok);
 }
+
+void NasalSysTests::testCommands()
+{
+    auto nasalSys = globals->get_subsystem<FGNasalSys>();
+    nasalSys->getAndClearErrorList();
+
+    fgSetInt("/foo/test", 7);
+    bool ok = FGTestApi::executeNasal(R"(
+     var f = func { 
+         var i = getprop('/foo/test');
+         setprop('foo/test', i + 4);
+     };
+                                      
+      addcommand('do-foo', f);
+      var ok = fgcommand('do-foo');
+      unitTest.assert(ok);
+    )");
+    CPPUNIT_ASSERT(ok);
+
+    CPPUNIT_ASSERT_EQUAL(11, fgGetInt("/foo/test"));
+
+    SGPropertyNode_ptr args(new SGPropertyNode);
+    ok = globals->get_commands()->execute("do-foo", args);
+    CPPUNIT_ASSERT(ok);
+    CPPUNIT_ASSERT_EQUAL(15, fgGetInt("/foo/test"));
+
+    ok = FGTestApi::executeNasal(R"(
+       var g = func { print('fail'); };
+       addcommand('do-foo', g);
+    )");
+    CPPUNIT_ASSERT(ok);
+    auto errors = nasalSys->getAndClearErrorList();
+    CPPUNIT_ASSERT_EQUAL(1UL, errors.size());
+
+    // old command shoudl still be registered and work
+    ok = globals->get_commands()->execute("do-foo", args);
+    CPPUNIT_ASSERT(ok);
+    CPPUNIT_ASSERT_EQUAL(19, fgGetInt("/foo/test"));
+
+    ok = FGTestApi::executeNasal(R"(
+      removecommand('do-foo');
+   )");
+    CPPUNIT_ASSERT(ok);
+
+    ok = FGTestApi::executeNasal(R"(
+     var ok = fgcommand('do-foo');
+     unitTest.assert(!ok);
+  )");
+    CPPUNIT_ASSERT(ok);
+    errors = nasalSys->getAndClearErrorList();
+    CPPUNIT_ASSERT_EQUAL(0UL, errors.size());
+
+    // should fail, command is removed
+    ok = globals->get_commands()->execute("do-foo", args);
+    CPPUNIT_ASSERT(!ok);
+    CPPUNIT_ASSERT_EQUAL(19, fgGetInt("/foo/test"));
+}

test_suite/unit_tests/Scripting/testNasalSys.hxx

@@ -32,6 +32,7 @@ class NasalSysTests : public CppUnit::TestFixture
     // Set up the test suite.
     CPPUNIT_TEST_SUITE(NasalSysTests);
     CPPUNIT_TEST(testStructEquality);
+    CPPUNIT_TEST(testCommands);
     CPPUNIT_TEST_SUITE_END();
 
 public:
@@ -43,6 +44,7 @@ public:
 
     // The tests.
     void testStructEquality();
+    void testCommands();
 };
 
 #endif  // _FG_NASALSYS_UNIT_TESTS_HXX