Test with Valgrind, stop leaking, more memory safety

.travis.yml

@@ -17,6 +17,10 @@ matrix:
       env: GMOCK_PATH=/usr/src/gmock
     - os: osx
       compiler: gcc #does anyone on osx use it?
+  include:
+    - os: linux
+      compiler: gcc
+      env: GMOCK_VER=1.8.0 VALGRIND_TESTS=ON
 
 addons:
   apt:
@@ -30,6 +34,7 @@ addons:
       - libboost-test-dev
       - google-mock
       - ninja-build
+      - valgrind
 
 before_install:
   - if [[ "${TRAVIS_OS_NAME}" = "osx" ]]; then brew update && brew install ninja; fi

CMakeLists.txt

@@ -12,6 +12,7 @@ set(CUKE_DISABLE_E2E_TESTS OFF CACHE BOOL "Disable end-to-end tests")
 set(CUKE_ENABLE_EXAMPLES OFF CACHE BOOL "Enable the examples")
 set(GMOCK_SRC_DIR "" CACHE STRING "Google Mock framework sources path (otherwise downloaded)")
 set(GMOCK_VER "1.7.0" CACHE STRING "Google Mock framework version to be used")
+option(VALGRIND_TESTS "Run tests within Valgrind" OFF)
 
 set(CMAKE_MODULE_PATH ${PROJECT_SOURCE_DIR}/cmake/modules)
 
@@ -102,6 +103,52 @@ if(NOT CUKE_DISABLE_GTEST)
     endif()
 endif()
 
+#
+# Valgrind
+#
+
+if(VALGRIND_TESTS)
+    find_program(VALGRIND_CMD valgrind)
+    if(NOT VALGRIND_CMD)
+        message(SEND_ERROR "Failed to find 'valgrind', cannot run tests with it as requested")
+    endif()
+    set(VALGRIND_ARGS --error-exitcode=2 --leak-check=full --show-leak-kinds=all --undef-value-errors=no)
+    function(add_test name)
+        if(NOT name STREQUAL "NAME")
+            _add_test(${VALGRIND_CMD} ${VALGRIND_ARGS} ${ARGV})
+            return()
+        endif()
+
+        set(TEST_ARGS ${ARGV})
+        list(FIND TEST_ARGS COMMAND COMMAND_IDX)
+        if(COMMAND_IDX EQUAL -1)
+            message(AUTHOR_WARNING "Weird command-line given to add_test(), not injecting valgrind")
+            _add_test(${ARGV})
+            return()
+        endif()
+
+        # We want to operate on the COMMAND, not the 'COMMAND' keyword preceding it
+        math(EXPR COMMAND_IDX "${COMMAND_IDX} + 1")
+
+        # Keep add_test() behaviour of replacing COMMANDs, when executable targets, with their output files
+        list(GET TEST_ARGS ${COMMAND_IDX} COMMAND)
+        if(TARGET ${COMMAND})
+            get_target_property(COMMAND_TYPE ${COMMAND} TYPE)
+            if(COMMAND_TYPE STREQUAL "EXECUTABLE")
+                # Inserting first, removing the original only after that, because inserting to the end of the list doesn't work
+                math(EXPR ORIG_COMMAND_IDX "${COMMAND_IDX} + 1")
+                list(INSERT TEST_ARGS ${COMMAND_IDX} "$<TARGET_FILE:${COMMAND}>")
+                list(REMOVE_AT TEST_ARGS ${ORIG_COMMAND_IDX})
+            endif()
+        endif()
+
+        # Insert the valgrind command line, before the command to execute
+        list(INSERT TEST_ARGS ${COMMAND_IDX} ${VALGRIND_CMD} ${VALGRIND_ARGS})
+
+        _add_test(${TEST_ARGS})
+    endfunction()
+endif()
+
 #
 # Cucumber-Cpp
 #

include/cucumber-cpp/internal/ContextManager.hpp

@@ -3,6 +3,7 @@
 
 #include <vector>
 
+#include <boost/make_shared.hpp>
 #include <boost/shared_ptr.hpp>
 #include <boost/weak_ptr.hpp>
 
@@ -26,7 +27,7 @@ class ContextManager {
 
 template<class T>
 weak_ptr<T> ContextManager::addContext() {
-    shared_ptr<T> shared(new T);
+    shared_ptr<T> shared(boost::make_shared<T>());
     contexts.push_back(shared);
     return weak_ptr<T> (shared);
 }

include/cucumber-cpp/internal/CukeCommands.hpp

@@ -26,7 +26,7 @@ class CukeCommands {
 	CukeCommands();
 	virtual ~CukeCommands();
 
-    void beginScenario(const TagExpression::tag_list *tags);
+    void beginScenario(const TagExpression::tag_list& tags = TagExpression::tag_list());
     void endScenario();
     const std::string snippetText(const std::string stepKeyword, const std::string stepName) const;
     MatchResult stepMatches(const std::string description) const;

include/cucumber-cpp/internal/Scenario.hpp

@@ -8,11 +8,11 @@ namespace internal {
 
 class Scenario {
 public:
-    Scenario(const TagExpression::tag_list *pTags);
+    Scenario(const TagExpression::tag_list& tags = TagExpression::tag_list());
 
     const TagExpression::tag_list & getTags();
 private:
-    shared_ptr<const TagExpression::tag_list> pTags;
+    const TagExpression::tag_list tags;
 };
 
 }

include/cucumber-cpp/internal/connectors/wire/WireProtocol.hpp

@@ -3,6 +3,7 @@
 
 #include "ProtocolHandler.hpp"
 #include "../../CukeEngine.hpp"
+#include <boost/shared_ptr.hpp>
 
 namespace cucumber {
 namespace internal {
@@ -17,14 +18,14 @@ class WireResponse {
 public:
     WireResponse() {};
 
-    virtual void accept(WireResponseVisitor *visitor) const = 0;
+    virtual void accept(WireResponseVisitor& visitor) const = 0;
 
     virtual ~WireResponse() {};
 };
 
 class SuccessResponse : public WireResponse {
 public:
-    void accept(WireResponseVisitor *visitor) const;
+    void accept(WireResponseVisitor& visitor) const;
 };
 
 class FailureResponse : public WireResponse {
@@ -37,7 +38,7 @@ class FailureResponse : public WireResponse {
     const std::string getMessage() const;
     const std::string getExceptionType() const;
 
-    void accept(WireResponseVisitor *visitor) const;
+    void accept(WireResponseVisitor& visitor) const;
 };
 
 class PendingResponse : public WireResponse {
@@ -49,7 +50,7 @@ class PendingResponse : public WireResponse {
 
     const std::string getMessage() const;
 
-    void accept(WireResponseVisitor *visitor) const;
+    void accept(WireResponseVisitor& visitor) const;
 };
 
 class StepMatchesResponse : public WireResponse {
@@ -60,7 +61,7 @@ class StepMatchesResponse : public WireResponse {
     StepMatchesResponse(const std::vector<StepMatch> & matchingSteps);
     const std::vector<StepMatch>& getMatchingSteps() const;
 
-    void accept(WireResponseVisitor *visitor) const;
+    void accept(WireResponseVisitor& visitor) const;
 };
 
 class SnippetTextResponse : public WireResponse {
@@ -72,16 +73,16 @@ class SnippetTextResponse : public WireResponse {
 
     const std::string getStepSnippet() const;
 
-    void accept(WireResponseVisitor *visitor) const;
+    void accept(WireResponseVisitor& visitor) const;
 };
 
 class WireResponseVisitor {
 public:
-    virtual void visit(const SuccessResponse *response) = 0;
-    virtual void visit(const FailureResponse *response) = 0;
-    virtual void visit(const PendingResponse *response) = 0;
-    virtual void visit(const StepMatchesResponse *response) = 0;
-    virtual void visit(const SnippetTextResponse *response) = 0;
+    virtual void visit(const SuccessResponse& response) = 0;
+    virtual void visit(const FailureResponse& response) = 0;
+    virtual void visit(const PendingResponse& response) = 0;
+    virtual void visit(const StepMatchesResponse& response) = 0;
+    virtual void visit(const SnippetTextResponse& response) = 0;
 
     virtual ~WireResponseVisitor() {};
 };
@@ -99,7 +100,7 @@ class WireCommand {
      *
      * @return The command response (ownership passed to the caller)
      */
-    virtual WireResponse *run(CukeEngine *engine) const = 0;
+    virtual boost::shared_ptr<WireResponse> run(CukeEngine& engine) const = 0;
 
     virtual ~WireCommand() {};
 };
@@ -133,7 +134,7 @@ class WireMessageCodec {
      *
      * @throws WireMessageCodecException
      */
-    virtual WireCommand *decode(const std::string &request) const = 0;
+    virtual boost::shared_ptr<WireCommand> decode(const std::string &request) const = 0;
 
     /**
      * Encodes a response to wire format.
@@ -142,7 +143,7 @@ class WireMessageCodec {
      *
      * @return The encoded string
      */
-    virtual const std::string encode(const WireResponse *response) const = 0;
+    virtual const std::string encode(const WireResponse& response) const = 0;
 
     virtual ~WireMessageCodec() {};
 };
@@ -153,8 +154,8 @@ class WireMessageCodec {
 class JsonSpiritWireMessageCodec : public WireMessageCodec {
 public:
     JsonSpiritWireMessageCodec();
-    WireCommand *decode(const std::string &request) const;
-    const std::string encode(const WireResponse *response) const;
+    boost::shared_ptr<WireCommand> decode(const std::string &request) const;
+    const std::string encode(const WireResponse& response) const;
 };
 
 /**
@@ -163,11 +164,11 @@ class JsonSpiritWireMessageCodec : public WireMessageCodec {
  */
 class WireProtocolHandler : public ProtocolHandler {
 private:
-    const WireMessageCodec *codec;
-    CukeEngine *engine;
+    const WireMessageCodec& codec;
+    CukeEngine& engine;
 
 public:
-    WireProtocolHandler(const WireMessageCodec *codec, CukeEngine *engine);
+    WireProtocolHandler(const WireMessageCodec& codec, CukeEngine& engine);
 
     std::string handle(const std::string &request) const;
 };

include/cucumber-cpp/internal/connectors/wire/WireProtocolCommands.hpp

@@ -2,31 +2,32 @@
 #define CUKE_WIREPROTOCOL_COMMANDS_HPP_
 
 #include "WireProtocol.hpp"
+#include <boost/shared_ptr.hpp>
 
 namespace cucumber {
 namespace internal {
 
 class ScenarioCommand : public WireCommand {
 protected:
-    std::auto_ptr<const CukeEngine::tags_type> tags;
+    const CukeEngine::tags_type tags;
 
-    ScenarioCommand(const CukeEngine::tags_type *tags);
+    ScenarioCommand(const CukeEngine::tags_type& tags);
 };
 
 
 class BeginScenarioCommand : public ScenarioCommand {
 public:
-    BeginScenarioCommand(const CukeEngine::tags_type *tags);
+    BeginScenarioCommand(const CukeEngine::tags_type& tags);
 
-    WireResponse *run(CukeEngine *engine) const;
+    boost::shared_ptr<WireResponse> run(CukeEngine& engine) const;
 };
 
 
 class EndScenarioCommand : public ScenarioCommand {
 public:
-    EndScenarioCommand(const CukeEngine::tags_type *tags);
+    EndScenarioCommand(const CukeEngine::tags_type& tags);
 
-    WireResponse *run(CukeEngine *engine) const;
+    boost::shared_ptr<WireResponse> run(CukeEngine& engine) const;
 };
 
 
@@ -37,22 +38,22 @@ class StepMatchesCommand : public WireCommand {
 public:
     StepMatchesCommand(const std::string & stepName);
 
-    WireResponse *run(CukeEngine *engine) const;
+    boost::shared_ptr<WireResponse> run(CukeEngine& engine) const;
 };
 
 
 class InvokeCommand : public WireCommand {
 private:
     const std::string stepId;
-    std::auto_ptr<const CukeEngine::invoke_args_type> args;
-    std::auto_ptr<const CukeEngine::invoke_table_type> tableArg;
+    const CukeEngine::invoke_args_type args;
+    const CukeEngine::invoke_table_type tableArg;
 
 public:
     InvokeCommand(const std::string & stepId,
-                  const CukeEngine::invoke_args_type *args,
-                  const CukeEngine::invoke_table_type * tableArg);
+                  const CukeEngine::invoke_args_type& args,
+                  const CukeEngine::invoke_table_type& tableArg);
 
-    WireResponse *run(CukeEngine *engine) const;
+    boost::shared_ptr<WireResponse> run(CukeEngine& engine) const;
 };
 
 
@@ -65,13 +66,13 @@ class SnippetTextCommand : public WireCommand {
                        const std::string & name,
                        const std::string & multilineArgClass);
 
-    WireResponse *run(CukeEngine *engine) const;
+    boost::shared_ptr<WireResponse> run(CukeEngine& engine) const;
 };
 
 
 class FailingCommand : public WireCommand {
 public:
-    WireResponse *run(CukeEngine *engine) const;
+    boost::shared_ptr<WireResponse> run(CukeEngine& engine) const;
 };
 
 }