Revert "Fixed Unit Tests to build reliable on docker environment"

CMakeLists.txt

@@ -245,7 +245,7 @@ if( CMAKE_BUILD_FLAG STREQUAL "UnitTests" )
     # Turn off link time optimisation to speed up compilation
     set(CMAKE_INTERPROCEDURAL_OPTIMIZATION FALSE)
 
-    include( cmake/build_google_test.cmake )
+    include( cmake/googletest.cmake )
 
     add_subdirectory( tests/common EXCLUDE_FROM_ALL )
     add_subdirectory( tests/unittests EXCLUDE_FROM_ALL )
@@ -258,7 +258,7 @@ if( CMAKE_BUILD_FLAG STREQUAL "ComponentTests" )
     # Turn off link time optimisation to speed up compilation
     set(CMAKE_INTERPROCEDURAL_OPTIMIZATION FALSE)
 
-    include( cmake/build_google_test.cmake )
+    include( cmake/googletest.cmake )
 
     add_subdirectory( tests/common EXCLUDE_FROM_ALL )
     add_subdirectory( tests/componenttests EXCLUDE_FROM_ALL )

cmake/build_google_test.cmake → cmake/googletest.cmake

@@ -93,22 +93,11 @@ macro( add_gtests TESTNAME )
 
     # gtest_discover_tests replaces gtest_add_tests,
     # see https://cmake.org/cmake/help/v3.10/module/GoogleTest.html for more options to pass to it
-    # DISCOVERY_MODE PRE_TEST defers test enumeration from cmake configure
-    # time to just before test execution. It was added in CMake 3.18; on older
-    # versions we omit it so configuration still works (falling back to the
-    # default POST_BUILD discovery mode).
-    if( CMAKE_VERSION VERSION_GREATER_EQUAL "3.18" )
-        gtest_discover_tests( ${TESTNAME}
-                # set a working directory so your project root so that you can find
-                # test data via paths relative to the project root
-                WORKING_DIRECTORY ${PROJECT_DIR}
-                DISCOVERY_MODE PRE_TEST
-                )
-    else()
-        gtest_discover_tests( ${TESTNAME}
-                WORKING_DIRECTORY ${PROJECT_DIR}
-                )
-    endif()
+    gtest_discover_tests( ${TESTNAME}
+            # set a working directory so your project root so that you can find
+            # test data via paths relative to the project root
+            WORKING_DIRECTORY ${PROJECT_DIR}
+            )
 
     set_target_properties( ${TESTNAME} PROPERTIES FOLDER test )
 

media/server/gstplayer/include/GstGenericPlayer.h

@@ -28,6 +28,7 @@
 #include "IGstGenericPlayer.h"
 #include "IGstGenericPlayerPrivate.h"
 #include "IGstInitialiser.h"
+#include "IGstProfiler.h"
 #include "IGstProtectionMetadataHelperFactory.h"
 #include "IGstSrc.h"
 #include "IGstWrapper.h"
@@ -58,11 +59,11 @@ class GstGenericPlayerFactory : public IGstGenericPlayerFactory
      */
     static std::weak_ptr<IGstGenericPlayerFactory> m_factory;
 
-    std::unique_ptr<IGstGenericPlayer> createGstGenericPlayer(
-        IGstGenericPlayerClient *client, IDecryptionService &decryptionService, MediaType type,
-        const VideoRequirements &videoRequirements, bool isLive,
-        const std::shared_ptr<firebolt::rialto::wrappers::IRdkGstreamerUtilsWrapperFactory> &rdkGstreamerUtilsWrapperFactory,
-        const std::shared_ptr<IGstProfilerFactory> &gstProfilerFactory) override;
+    std::unique_ptr<IGstGenericPlayer>
+    createGstGenericPlayer(IGstGenericPlayerClient *client, IDecryptionService &decryptionService, MediaType type,
+                           const VideoRequirements &videoRequirements, bool isLive,
+                           const std::shared_ptr<firebolt::rialto::wrappers::IRdkGstreamerUtilsWrapperFactory>
+                               &rdkGstreamerUtilsWrapperFactory) override;
 };
 
 /**

media/server/gstplayer/interface/IGstGenericPlayer.h

@@ -28,7 +28,6 @@
 #include "IDataReader.h"
 #include "IDecryptionService.h"
 #include "IGstGenericPlayerClient.h"
-#include "IGstProfiler.h"
 #include "IHeartbeatHandler.h"
 #include "IMediaPipeline.h"
 #include "IRdkGstreamerUtilsWrapper.h"
@@ -54,25 +53,21 @@ class IGstGenericPlayerFactory
     static std::shared_ptr<IGstGenericPlayerFactory> getFactory();
 
     /**
-     * @brief Creates an IGstGenericPlayer object.
+     * @brief Creates a IGstGenericPlayer object.
      *
-     * @param[in] client                       : The gstreamer player client.
-     * @param[in] decryptionService            : The decryption service.
-     * @param[in] type                         : The media type the gstreamer player shall support.
-     * @param[in] videoRequirements            : The video requirements for the playback.
-     * @param[in] isLive                       : Indicates if the media is live.
-     * @param[in] rdkGstreamerUtilsWrapperFactory : The rdk gstreamer utils wrapper factory.
-     * @param[in] gstProfilerFactory           : The gst profiler factory. Defaults to nullptr; when null, the
-     *                                                concrete factory falls back to IGstProfilerFactory::getFactory().
-     *                                                This avoids evaluating the real singleton at mocked call sites.
+     * @param[in] client            : The gstreamer player client.
+     * @param[in] decryptionService : The decryption service.
+     * @param[in] type              : The media type the gstreamer player shall support.
+     * @param[in] videoRequirements : The video requirements for the playback.
+     * @param[in] isLive            : Indicates if the media is live.
      *
      * @retval the new player instance or null on error.
      */
-    virtual std::unique_ptr<IGstGenericPlayer> createGstGenericPlayer(
-        IGstGenericPlayerClient *client, IDecryptionService &decryptionService, MediaType type,
-        const VideoRequirements &videoRequirements, bool isLive,
-        const std::shared_ptr<firebolt::rialto::wrappers::IRdkGstreamerUtilsWrapperFactory> &rdkGstreamerUtilsWrapperFactory,
-        const std::shared_ptr<IGstProfilerFactory> &gstProfilerFactory = nullptr) = 0;
+    virtual std::unique_ptr<IGstGenericPlayer>
+    createGstGenericPlayer(IGstGenericPlayerClient *client, IDecryptionService &decryptionService, MediaType type,
+                           const VideoRequirements &videoRequirements, bool isLive,
+                           const std::shared_ptr<firebolt::rialto::wrappers::IRdkGstreamerUtilsWrapperFactory>
+                               &rdkGstreamerUtilsWrapperFactory) = 0;
 };
 
 class IGstGenericPlayer

media/server/gstplayer/source/GstGenericPlayer.cpp

@@ -84,8 +84,7 @@ std::shared_ptr<IGstGenericPlayerFactory> IGstGenericPlayerFactory::getFactory()
 std::unique_ptr<IGstGenericPlayer> GstGenericPlayerFactory::createGstGenericPlayer(
     IGstGenericPlayerClient *client, IDecryptionService &decryptionService, MediaType type,
     const VideoRequirements &videoRequirements, bool isLive,
-    const std::shared_ptr<firebolt::rialto::wrappers::IRdkGstreamerUtilsWrapperFactory> &rdkGstreamerUtilsWrapperFactory,
-    const std::shared_ptr<IGstProfilerFactory> &gstProfilerFactory)
+    const std::shared_ptr<firebolt::rialto::wrappers::IRdkGstreamerUtilsWrapperFactory> &rdkGstreamerUtilsWrapperFactory)
 {
     std::unique_ptr<IGstGenericPlayer> gstPlayer;
 
@@ -110,21 +109,10 @@ std::unique_ptr<IGstGenericPlayer> GstGenericPlayerFactory::createGstGenericPlay
             throw std::runtime_error("Cannot create RdkGstreamerUtilsWrapper");
         }
 
-        // Fall back to the default profiler factory if the caller explicitly
-        // passed nullptr. IGstProfilerFactory::getFactory() can itself return
-        // nullptr on allocation failure, so re-check afterwards and fail with
-        // a clear error rather than passing null on and reporting the generic
-        // "No gst profiler factory provided" message from the constructor.
-        auto resolvedGstProfilerFactory = gstProfilerFactory ? gstProfilerFactory : IGstProfilerFactory::getFactory();
-        if (!resolvedGstProfilerFactory)
-        {
-            throw std::runtime_error("Cannot obtain default IGstProfilerFactory");
-        }
-
         gstPlayer = std::make_unique<
             GstGenericPlayer>(client, decryptionService, type, videoRequirements, isLive, gstWrapper, glibWrapper,
                               rdkGstreamerUtilsWrapper, IGstInitialiser::instance(), std::make_unique<FlushWatcher>(),
-                              IGstSrcFactory::getFactory(), resolvedGstProfilerFactory,
+                              IGstSrcFactory::getFactory(), IGstProfilerFactory::getFactory(),
                               common::ITimerFactory::getFactory(),
                               std::make_unique<GenericPlayerTaskFactory>(client, gstWrapper, glibWrapper,
                                                                          rdkGstreamerUtilsWrapper,

scripts/gtest/build_and_run_tests.py

@@ -30,60 +30,11 @@
 valgrindOutput = "valgrind_report"
 valgrindIgnore = "rialto.supp"
 
-# Argparse type helper: only accept integers >= 1. Values like 0 (make treats
-# as unlimited jobs) or negatives would defeat the purpose of --jobs on
-# memory-constrained hosts.
-def positiveInt(value):
-    try:
-        intValue = int(value)
-    except ValueError:
-        raise argparse.ArgumentTypeError(f"must be an integer >= 1, got {value!r}")
-    if intValue < 1:
-        raise argparse.ArgumentTypeError(f"must be >= 1, got {value}")
-    return intValue
-
-# Argparse type helper: only accept a non-empty, relative path that does not
-# escape the working directory, either lexically or via symlinks. This
-# protects the destructive --clean path from unsafe --output values (empty
-# string, absolute path, ".", "..", or a symlink into another location).
-# Fails at parse time with a clean CLI message rather than a Python traceback.
-def safeRelativePath(value):
-    if not value:
-        raise argparse.ArgumentTypeError("must not be empty")
-    if os.path.isabs(value):
-        raise argparse.ArgumentTypeError(f"must be a relative path, got {value!r}")
-    normalised = os.path.normpath(value)
-    # os.path.normpath collapses internal ".." segments, so a normalised
-    # path escapes the working directory iff it equals ".." or starts with
-    # ".." + separator. We also reject paths that collapse to "." (e.g. "."
-    # itself or "foo/.."), since those would target the working directory.
-    if normalised == ".":
-        raise argparse.ArgumentTypeError(f"must not be the working directory, got {value!r}")
-    if normalised == ".." or normalised.startswith(".." + os.sep):
-        raise argparse.ArgumentTypeError(f"must not escape the working directory, got {value!r}")
-    # Lexical checks above are not enough on their own: a symlink whose
-    # target is outside the working directory would still let "rm -rf"
-    # escape. Resolve the path (following existing symlinks) and confirm
-    # it lives strictly under cwd. If the path does not exist yet, realpath
-    # returns the canonical absolute form, which is still safe.
-    cwd = os.path.realpath(os.getcwd())
-    resolved = os.path.realpath(normalised)
-    if resolved == cwd:
-        raise argparse.ArgumentTypeError(f"must not resolve to the working directory, got {value!r}")
-    try:
-        common = os.path.commonpath([resolved, cwd])
-    except ValueError:
-        # e.g. different drives on Windows
-        raise argparse.ArgumentTypeError(f"must resolve inside the working directory, got {value!r}")
-    if common != cwd:
-        raise argparse.ArgumentTypeError(f"must resolve inside the working directory, got {value!r}")
-    return normalised
-
 # Get the arguments supported by the googletest script
 def getGenericArguments(argParser, suiteInfo):
     # Get arguments
-    argParser.add_argument("-o", "--output", type=safeRelativePath, default=safeRelativePath("build"),
-                        help="Location to write the build files to (default 'build'). Must be a relative path.")
+    argParser.add_argument("-o", "--output", default="build",
+                        help="Location to write the build files to (default 'build').")
     argParser.add_argument("-f", "--file", nargs='?', const="",
                         help="Write the build and test output to a file (default '" + getDefaultResultsOutputFileName() + ".log') \n" \
                              + "Valgrind output also written to file, default file name only \n" \
@@ -112,8 +63,6 @@ def getGenericArguments(argParser, suiteInfo):
                              + "Note: Valgrind can only write output to one source (log or xml). \n" \
                              + "Note: Requires version valgrind 3.17.0+ installed. \n")
     argParser.add_argument("-cov", "--coverage", action='store_true', help="Generates the full coverage report")
-    argParser.add_argument("-j", "--jobs", type=positiveInt, default=multiprocessing.cpu_count(),
-                        help="Number of parallel make jobs, must be >= 1 (default: number of CPUs).")
 
 # Builds and runs googletests for the given suites
 def buildAndRunGTests(args, f, buildDefines, suitesToRun):
@@ -124,16 +73,12 @@ def buildAndRunGTests(args, f, buildDefines, suitesToRun):
     os.environ["RIALTO_CONSOLE_LOG"] = "1"
     # Set env variable to enable debug prints
     os.environ["RIALTO_DEBUG"] = "5"
+    # Enable profiler for all test suites.
+    os.environ["PROFILER_ENABLED"] = "true"
 
-    # Clean if required. args['output'] is validated at CLI parse time by
-    # safeRelativePath(), which enforces both lexical and symlink-resolved
-    # containment inside the working directory, so "rm -rf" here cannot
-    # target an unsafe location. The "--" terminator prevents any value
-    # starting with "-" (which the validator would accept as a relative
-    # path) from being misinterpreted by rm as an option like
-    # --no-preserve-root.
+    # Clean if required
     if args['clean'] == True:
-        executeCmd = ["rm", "-rf", "--", args['output'], valgrindOutput + ".log"]
+        executeCmd = ["rm", "-rf", args['output'], valgrindOutput + ".log"]
         runcmd(executeCmd, cwd=os.getcwd())
 
     # Get xml output file name if any
@@ -147,7 +92,7 @@ def buildAndRunGTests(args, f, buildDefines, suitesToRun):
 
     # Build the test executables
     if args['noBuild'] == False:
-        buildTargets(suitesToRun, buildDefines, args['output'], f, args['valgrind'], args['coverage'], args['jobs'])
+        buildTargets(suitesToRun, buildDefines, args['output'], f, args['valgrind'], args['coverage'])
 
     # Run the tests with the optional settings
     if args['noTest'] == False:
@@ -156,7 +101,7 @@ def buildAndRunGTests(args, f, buildDefines, suitesToRun):
 
 
 # Build the target executables
-def buildTargets (suites, buildDefines, outputDir, resultsFile, debug, coverage, jobs=multiprocessing.cpu_count()):
+def buildTargets (suites, buildDefines, outputDir, resultsFile, debug, coverage):
     # Run cmake
     cmakeCmd = ["cmake", "-B", outputDir]
     for define in buildDefines:
@@ -168,7 +113,7 @@ def buildTargets (suites, buildDefines, outputDir, resultsFile, debug, coverage,
     runcmd(cmakeCmd, cwd=os.getcwd())
 
     # Make targets
-    jarg = "-j" + str(jobs)
+    jarg = "-j" + str(multiprocessing.cpu_count())
     makeCmd = ["make", jarg]
     for key in suites:
         makeCmd.append(suites[key]["suite"])

tests/unittests/media/server/gstplayer/genericPlayer/CreateTest.cpp

@@ -152,20 +152,21 @@ TEST_F(RialtoServerCreateGstGenericPlayerTest, FactoryCreatesObject)
     expectSetSignalCallbacks();
     expectSetUri();
     expectCheckPlaySink();
-    expectCreateProfiler();
     EXPECT_CALL(*m_gstWrapperMock, gstElementSetState(&m_pipeline, GST_STATE_READY))
         .WillOnce(Return(GST_STATE_CHANGE_SUCCESS));
+    EXPECT_CALL(*m_gstWrapperMock, gstObjectRef(&m_pipeline)).WillOnce(Return(&m_pipeline));
+
     std::shared_ptr<firebolt::rialto::server::IGstGenericPlayerFactory> factory =
         firebolt::rialto::server::IGstGenericPlayerFactory::getFactory();
     ASSERT_NE(factory, nullptr);
-    auto player{factory->createGstGenericPlayer(&m_gstPlayerClient, m_decryptionServiceMock, m_type, m_videoReq, m_kIsLive,
-                                                m_rdkGstreamerUtilsWrapperFactoryMock, m_gstProfilerFactoryMock)};
+    auto player{factory->createGstGenericPlayer(&m_gstPlayerClient, m_decryptionServiceMock, m_type, m_videoReq,
+                                                m_kIsLive, m_rdkGstreamerUtilsWrapperFactoryMock)};
     EXPECT_NE(player, nullptr);
 
     // Destroy expectations
     EXPECT_CALL(*m_gstWrapperMock, gstBusSetSyncHandler(nullptr, nullptr, nullptr, nullptr));
     EXPECT_CALL(*m_gstWrapperMock, gstElementSetState(_, GST_STATE_NULL)).WillOnce(Return(GST_STATE_CHANGE_SUCCESS));
-    EXPECT_CALL(*m_gstWrapperMock, gstObjectUnref(_)).Times(2);
+    EXPECT_CALL(*m_gstWrapperMock, gstObjectUnref(_)).Times(3);
     EXPECT_CALL(*m_glibWrapperMock, gThreadPoolStopUnusedThreads());
     player.reset();
 

tests/unittests/media/server/main/mediaPipeline/LoadTest.cpp

@@ -47,7 +47,7 @@ TEST_F(RialtoServerMediaPipelineLoadTest, Success)
     mainThreadWillEnqueueTaskAndWait();
     mainThreadWillEnqueueTask();
     EXPECT_CALL(*m_gstPlayerFactoryMock,
-                createGstGenericPlayer(_, _, m_type, VideoRequirementsMatcher(m_videoReq), m_kIsLive, _, _))
+                createGstGenericPlayer(_, _, m_type, VideoRequirementsMatcher(m_videoReq), m_kIsLive, _))
         .WillOnce(Return(ByMove(std::move(m_gstPlayer))));
     EXPECT_CALL(*m_mediaPipelineClientMock, notifyNetworkState(NetworkState::BUFFERING));
 
@@ -62,7 +62,7 @@ TEST_F(RialtoServerMediaPipelineLoadTest, CreateGstPlayerFailure)
 {
     mainThreadWillEnqueueTaskAndWait();
     EXPECT_CALL(*m_gstPlayerFactoryMock,
-                createGstGenericPlayer(_, _, m_type, VideoRequirementsMatcher(m_videoReq), m_kIsLive, _, _))
+                createGstGenericPlayer(_, _, m_type, VideoRequirementsMatcher(m_videoReq), m_kIsLive, _))
         .WillOnce(Return(ByMove(nullptr)));
     EXPECT_CALL(*m_mediaPipelineClientMock, notifyNetworkState(_)).Times(0);
 

tests/unittests/media/server/main/mediaPipeline/base/MediaPipelineTestBase.cpp

@@ -86,7 +86,7 @@ void MediaPipelineTestBase::loadGstPlayer()
 {
     mainThreadWillEnqueueTaskAndWait();
     mainThreadWillEnqueueTask();
-    EXPECT_CALL(*m_gstPlayerFactoryMock, createGstGenericPlayer(_, _, _, _, _, _, _))
+    EXPECT_CALL(*m_gstPlayerFactoryMock, createGstGenericPlayer(_, _, _, _, _, _))
         .WillOnce(DoAll(SaveArg<0>(&m_gstPlayerCallback), Return(ByMove(std::move(m_gstPlayer)))));
     EXPECT_CALL(*m_mediaPipelineClientMock, notifyNetworkState(NetworkState::BUFFERING));
 

tests/unittests/media/server/mocks/gstplayer/GstGenericPlayerFactoryMock.h

@@ -36,8 +36,7 @@ class GstGenericPlayerFactoryMock : public IGstGenericPlayerFactory
                 (IGstGenericPlayerClient * client, IDecryptionService &decryptionService, MediaType type,
                  const VideoRequirements &videoRequirements, bool isLive,
                  const std::shared_ptr<firebolt::rialto::wrappers::IRdkGstreamerUtilsWrapperFactory>
-                     &rdkGstreamerUtilsWrapperFactory,
-                 const std::shared_ptr<IGstProfilerFactory> &gstProfilerFactory),
+                     &rdkGstreamerUtilsWrapperFactory),
                 (override));
 };
 } // namespace firebolt::rialto::server