Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Unit tests #362

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Unit tests #362

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
25bfb76
bare essential changes to get unit-tests working
pgruenbacher-TSUS Jul 9, 2019
956eb6d
fix library loading for ctest. fix renderbeast tests
pgruenbacher-TSUS Jul 9, 2019
1fe79c5
explanation for macro
pgruenbacher Jul 12, 2019
385dc9d
more test fixes
pgruenbacher-TSUS Jul 12, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,20 @@ include(Source/CMake/Properties.cmake)
include(Source/CMake/FindPackageOrBuild.cmake)
include(Source/CMake/HelperMethods.cmake)


set(BUILD_TESTS OFF CACHE BOOL "If true, build targets for running unit tests will be included in the output.")
if (BUILD_TESTS)
# Enables testing for this directory and below. See also the add_test
# command. Note that ctest expects to find a test file in the build
# directory root. Therefore, this command should be in the source
# directory root.
enable_testing()
endif()

add_subdirectory(Source)

# Options
set(SCRIPT_API "None" CACHE STRING "Which scripting API/language to use, if any. If no scripting API is chosen only native C++ bsf core will be built. If a scripting API is chosen, then a separate scripting layer on top of the core will be included in the build as well.")
set_property(CACHE SCRIPT_API PROPERTY STRINGS "None" "C#")

set(SCRIPT_BINDING_GENERATION OFF CACHE BOOL "If true, script binding generation will be supported through a specialized build target. Enable this if you plan on modifying the scripting API. Requires the SBGen tool dependency. Only relevant if you have selected a SCRIPT_API other than \"None\".")
set(SCRIPT_BINDING_GENERATION OFF CACHE BOOL "If true, script binding generation will be supported through a specialized build target. Enable this if you plan on modifying the scripting API. Requires the SBGen tool dependency. Only relevant if you have selected a SCRIPT_API other than \"None\".")
32 changes: 32 additions & 0 deletions Source/CMake/HelperMethods.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,38 @@ MACRO(end_find_package FOLDER_NAME MAIN_LIB_NAME)
set(${FOLDER_NAME}_INCLUDE_DIRS ${${FOLDER_NAME}_INCLUDE_DIR})
ENDMACRO()

# the purpose of bs_add_test is to allow for the local library path to be found by the tester.
# It's necessary to pass the environment variables because ctest will usually run with a clean environment.
# It was implemented to work for linux, as of July 2019 not tested on windows.
MACRO(bs_add_test)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the purpose of this macro vs. using add_test? Also, I'm seeing references to Debug and Release - does it work on other configs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i updated with commit for explanation. Esentially I was getting "found .so library" errors until I updated the tests. plus it's just useful to have add_test wrapper for future in case there's other common properties.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I honestly don't know if it works for other configs. But it at least works for my needs at the moment.

cmake_parse_arguments(
PARSED_ARGS # prefix of output variables
"" # list of names of the boolean arguments (only defined ones will be true)
"NAME" # list of names of mono-valued arguments
"" # multivalue args
${ARGN} # arguments of the function to parse, here we take the all original ones
)

add_test(${ARGV})

if(NOT PARSED_ARGS_NAME)
message(FATAL_ERROR "You must provide a name for bs_add_test (bs_add_test(NAME name)")
endif(NOT PARSED_ARGS_NAME)

IF(CMAKE_BUILD_TYPE MATCHES Debug)
# windows
set_tests_properties( ${PARSED_ARGS_NAME} PROPERTIES ENVIRONMENT "PATH=${CMAKE_RUNTIME_OUTPUT_DIRECTORY_DEBUG};$ENV{PATH}" )
# linux
set_tests_properties( ${PARSED_ARGS_NAME} PROPERTIES ENVIRONMENT "LD_LIBRARY_PATH=${CMAKE_RUNTIME_OUTPUT_DIRECTORY_DEBUG};" )
elseif (CMAKE_BUILD_TYPE MATCHES Release)
# windows
set_tests_properties( ${PARSED_ARGS_NAME} PROPERTIES ENVIRONMENT "PATH=${CMAKE_RUNTIME_OUTPUT_DIRECTORY_RELEASE};$ENV{PATH}" )
# linux
set_tests_properties( ${PARSED_ARGS_NAME} PROPERTIES ENVIRONMENT "LD_LIBRARY_PATH=${CMAKE_RUNTIME_OUTPUT_DIRECTORY_RELEASE};" )
endif()

ENDMACRO()

function(install_dependency_binary FILE_PATH CONFIG)
if(NOT EXISTS ${FILE_PATH})
return()
Expand Down
8 changes: 2 additions & 6 deletions Source/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,6 @@ set_property(CACHE RENDERER_MODULE PROPERTY STRINGS RenderBeast Null)

set(INCLUDE_ALL_IN_WORKFLOW OFF CACHE BOOL "If true, all libraries (even those not selected) will be included in the generated workflow (e.g. Visual Studio solution). This is useful when working on engine internals with a need for easy access to all parts of it. Only relevant for workflow generators like Visual Studio or XCode.")

set(BUILD_TESTS OFF CACHE BOOL "If true, build targets for running unit tests will be included in the output.")

set(BUILD_BSL OFF CACHE BOOL "If true, build lexer & parser for BSL. Requires flex & bison dependencies.")

set(BUILD_ALL_RENDER_API OFF CACHE BOOL "If true, all supported render backends will be built, regardless of choice in RENDER_API_MODULE. Choice in RENDER_API_MODULE will still be used as the default.")
Expand Down Expand Up @@ -208,8 +206,6 @@ add_subdirectory(Plugins/bsfSL)

## Tests
if(BUILD_TESTS)
enable_testing()

add_executable(UtilityTest
Foundation/bsfUtility/Private/UnitTests/BsUtilityTest.cpp
Foundation/bsfUtility/Private/UnitTests/BsUtilityTestSuite.cpp
Expand All @@ -228,8 +224,8 @@ if(BUILD_TESTS)
set_property(TARGET UtilityTest PROPERTY FOLDER Tests)
set_property(TARGET CoreTest PROPERTY FOLDER Tests)

add_test(NAME UtilityTests COMMAND $<TARGET_FILE:UtilityTest>)
add_test(NAME CoreTests COMMAND $<TARGET_FILE:UtilityTest>)
bs_add_test(NAME UtilityTests COMMAND $<TARGET_FILE:UtilityTest>)
bs_add_test(NAME CoreTests COMMAND $<TARGET_FILE:UtilityTest>)
endif()

## Builtin resource preprocessing
Expand Down
1 change: 1 addition & 0 deletions Source/Foundation/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ elseif(CMAKE_CXX_COMPILER_ID MATCHES "Clang" OR CMAKE_CXX_COMPILER_ID MATCHES "A
elseif("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU")
target_compile_options(bsf PUBLIC
-fno-exceptions
-msse4.1
$<$<COMPILE_LANGUAGE:CXX>:-fno-rtti>)
else()
# TODO_OTHER_COMPILERS_GO_HERE
Expand Down
6 changes: 3 additions & 3 deletions Source/Foundation/bsfCore/Private/UnitTests/BsCoreTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ namespace bs

void CoreTestSuite::testAnimCurveIntegration()
{
static constexpr float EPSILON = 0.0001f;
static constexpr float EPSILON = 0.001f;

// Construct some curves
TAnimationCurve<float> curveConstant(
Expand Down Expand Up @@ -113,7 +113,7 @@ namespace bs

void CoreTestSuite::testLookupTable()
{
static constexpr float EPSILON = 0.0001f;
static constexpr float EPSILON = 0.001f;

TAnimationCurve<Vector3> curve
({
Expand Down Expand Up @@ -155,4 +155,4 @@ int main()
tests->run(testOutput);

return 0;
}
}
2 changes: 1 addition & 1 deletion Source/Foundation/bsfUtility/Private/UnitTests/BsFileSystemTestSuite.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ namespace bs
FileSystem::remove(mTestDirectory, true);
if (FileSystem::exists(mTestDirectory))
{
LOGERR("FileSystemTestSuite failed to delete '" + mTestDirectory.toString()
BS_LOG(Error, FileSystem, "FileSystemTestSuite failed to delete '" + mTestDirectory.toString()
+ "', you should remove it manually.");
}
}
Expand Down
36 changes: 35 additions & 1 deletion Source/Foundation/bsfUtility/Private/UnitTests/BsUtilityTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,48 @@
#include "Testing/BsConsoleTestOutput.h"
#include "Private/UnitTests/BsUtilityTestSuite.h"

#include "BsApplication.h"
Copy link
Member

@BearishSun BearishSun Jul 12, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally I organize the non-plugin code in layers. 'Utility' layer is the lowest one, and it shouldn't be referencing anything from the higher layers (this includes Application or CoreApplication). Are these tests failing without the Application started up? Ideally we can run them without it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm. well we could just change it to BsCoreApplication, which is the core layer, and therefore compatible with all other layers of testing for most part. I'll make the change and see if that works.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nope nvm, CoreApplication can't be initialized by itself for running the utility tests. stuff like gProfiler still gets called

#include "BsEngineConfig.h"

using namespace bs;


START_UP_DESC testStartupDesc()
{
START_UP_DESC desc;

// Set up default plugins
desc.renderAPI = BS_RENDER_API_MODULE;
// desc.renderAPI = "NullRenderAPI"
desc.renderer = BS_RENDERER_MODULE;
// desc.renderer = "NullRenderer";
desc.audio = BS_AUDIO_MODULE;
desc.physics = BS_PHYSICS_MODULE;

desc.importers.push_back("bsfFreeImgImporter");
desc.importers.push_back("bsfFBXImporter");
desc.importers.push_back("bsfFontImporter");
desc.importers.push_back("bsfSL");

VideoMode mode;
desc.primaryWindowDesc.videoMode = mode;
desc.primaryWindowDesc.fullscreen = false;
desc.primaryWindowDesc.title = "test";
return desc;
}

int main()
{

auto desc = testStartupDesc();
bs::Application::startUp(desc);

SPtr<TestSuite> tests = UtilityTestSuite::create<UtilityTestSuite>();

ConsoleTestOutput testOutput;
tests->run(testOutput);

bs::Application::shutDown();

return 0;
}
}
2 changes: 1 addition & 1 deletion Source/Foundation/bsfUtility/Testing/BsTestSuite.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ namespace bs
for (auto& testEntry : mTests)
{
mActiveTestName = testEntry.name;

std::cout << mActiveTestName << std::endl;
(this->*(testEntry.test))();
}

Expand Down
13 changes: 6 additions & 7 deletions Source/Foundation/bsfUtility/Utility/BsOctree.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -380,16 +380,16 @@ namespace bs
public:
/** Initializes the iterator, starting with the root octree node. */
NodeIterator(const Octree& tree)
:mCurrentNode(HNode(&tree.mRoot, tree.mRootBounds)), mStackAlloc(), mNodeStack(&mStackAlloc)
:mCurrentNode(HNode(&tree.mRoot, tree.mRootBounds))
{
mNodeStack.push_back(mCurrentNode);
mNodeStack.add(mCurrentNode);
}

/** Initializes the iterator using a specific node and its bounds. */
NodeIterator(const Node* node, const NodeBounds& bounds)
:mCurrentNode(HNode(node, bounds)), mStackAlloc(), mNodeStack(&mStackAlloc)
:mCurrentNode(HNode(node, bounds))
{
mNodeStack.push_back(mCurrentNode);
mNodeStack.add(mCurrentNode);
}

/**
Expand Down Expand Up @@ -423,13 +423,12 @@ namespace bs
Node* childNode = mCurrentNode.getNode()->getChild(child);
NodeBounds childBounds = mCurrentNode.getBounds().getChild(child);

mNodeStack.emplace_back(childNode, childBounds);
mNodeStack.add({childNode, childBounds});
}

private:
HNode mCurrentNode;
StaticAlloc<Options::MaxDepth * 8 * sizeof(HNode), FreeAlloc> mStackAlloc;
StaticVector<HNode, Options::MaxDepth * 8> mNodeStack;
SmallVector<HNode, Options::MaxDepth * 8> mNodeStack;
};

/** Iterator that iterates over all elements in a single node. */
Expand Down
13 changes: 6 additions & 7 deletions Source/Foundation/bsfUtility/Utility/BsQuadtree.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -372,16 +372,16 @@ namespace bs
public:
/** Initializes the iterator, starting with the root quadtree node. */
NodeIterator(const Quadtree& tree)
:mCurrentNode(HNode(&tree.mRoot, tree.mRootBounds)), mStackAlloc(), mNodeStack(&mStackAlloc)
:mCurrentNode(HNode(&tree.mRoot, tree.mRootBounds))
{
mNodeStack.push_back(mCurrentNode);
mNodeStack.add(mCurrentNode);
}

/** Initializes the iterator using a specific node and its bounds. */
NodeIterator(const Node* node, const NodeBounds& bounds)
:mCurrentNode(HNode(node, bounds)), mStackAlloc(), mNodeStack(&mStackAlloc)
:mCurrentNode(HNode(node, bounds))
{
mNodeStack.push_back(mCurrentNode);
mNodeStack.add(mCurrentNode);
}

/**
Expand Down Expand Up @@ -415,13 +415,12 @@ namespace bs
Node* childNode = mCurrentNode.getNode()->getChild(child);
NodeBounds childBounds = mCurrentNode.getBounds().getChild(child);

mNodeStack.emplace_back(childNode, childBounds);
mNodeStack.add({childNode, childBounds});
}

private:
HNode mCurrentNode;
StaticAlloc<Options::MaxDepth * 4 * sizeof(HNode), FreeAlloc> mStackAlloc;
StaticVector<HNode, Options::MaxDepth * 4> mNodeStack;
SmallVector<HNode, Options::MaxDepth * 4> mNodeStack;
};

/** Iterator that iterates over all elements in a single node. */
Expand Down
50 changes: 50 additions & 0 deletions Source/Plugins/bsfRenderBeast/BsRenderBeastTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
//************************************ bs::framework - Copyright 2018 Marko Pintera **************************************//
//*********** Licensed under the MIT license. See LICENSE.md for full terms. This notice is not to be removed. ***********//
#include "Testing/BsConsoleTestOutput.h"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to keep these non-unit tests elsewhere. Generally these things might require custom assets and might be bigger than one file as well, so it makes sense to me to dedicate a space for them. Even though this is a simple one I feel it fits into that category.

Perhaps we can add a Tests folder to bsfExamples repository? Or a custom repository.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also applies to BsUtilityTest.cpp above.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

eh i prefer tests to be close to the code of interest. otherwise they can too quickly go out of date.
unit-tests shouldn't be too dependent on assets. unit-tests != render tests != gameplay tests and can be separate. I'd prefer to be able to add a lot more tests to validate the serialization, threading, memory, perf. and math.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the issue with separate repos is that the most common example is a method signature changes, and then the author needs to remember to go to the separate repo, and update it there as well. Ideally the dev should just be compiling with build_tests=ON and so the tests are always compiling along with any signature changes.

Copy link
Contributor Author

@pgruenbacher pgruenbacher Jul 12, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

anyone compiling the source code should be running the unit-tests on their platform as well to validate. And I don't find files ending with *_test.cpp to be confusing when intermixed with source code. Having FileSystem.cpp and FileSystem_test.cpp in the same folder is a lot more intuitive than in a separate repo.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. Because you are starting up the application I assumed this was doing more than just a unit test.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah no, starting up the application was only necessary for starting some of the modules. some of the tests involved code that had to be initialized by the modules. At the very least Platform, MemStack, MessageHandler had to be startUp(). otherwise any tests using memstack will fail obviously.

#include "BsRenderBeastTestSuite.h"

#include "BsApplication.h"
#include "BsEngineConfig.h"

using namespace bs;


START_UP_DESC testStartupDesc()
{
START_UP_DESC desc;

// Set up default plugins
desc.renderAPI = BS_RENDER_API_MODULE;
// desc.renderAPI = "NullRenderAPI"
desc.renderer = BS_RENDERER_MODULE;
// desc.renderer = "NullRenderer";
desc.audio = BS_AUDIO_MODULE;
desc.physics = BS_PHYSICS_MODULE;

desc.importers.push_back("bsfFreeImgImporter");
desc.importers.push_back("bsfFBXImporter");
desc.importers.push_back("bsfFontImporter");
desc.importers.push_back("bsfSL");

VideoMode mode;
desc.primaryWindowDesc.videoMode = mode;
desc.primaryWindowDesc.fullscreen = false;
desc.primaryWindowDesc.title = "test";
return desc;
}

int main()
{

auto desc = testStartupDesc();
bs::Application::startUp(desc);

SPtr<TestSuite> tests = RenderBeastTestSuite::create<RenderBeastTestSuite>();

ConsoleTestOutput testOutput;
tests->run(testOutput);

bs::Application::shutDown();

return 0;
}
10 changes: 1 addition & 9 deletions Source/Plugins/bsfRenderBeast/BsRenderBeastTestSuite.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,10 @@
//*********** Licensed under the MIT license. See LICENSE.md for full terms. This notice is not to be removed. ***********//
#include "Testing/BsTestSuite.h"
#include "Utility/BsTextureRowAllocator.h"
#include "BsRenderBeastTestSuite.h"

namespace bs
{
/** Runs unit tests for systems specific to the RenderBeast plugin. */
class RenderBeastTestSuite : public TestSuite
{
public:
RenderBeastTestSuite();

private:
void testTextureRowAllocator();
};

RenderBeastTestSuite::RenderBeastTestSuite()
{
Expand Down
20 changes: 20 additions & 0 deletions Source/Plugins/bsfRenderBeast/BsRenderBeastTestSuite.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
//************************************ bs::framework - Copyright 2018 Marko Pintera **************************************//
//*********** Licensed under the MIT license. See LICENSE.md for full terms. This notice is not to be removed. ***********//
#pragma once

#include "Testing/BsTestSuite.h"

namespace bs
{

/** Runs unit tests for systems specific to the RenderBeast plugin. */
class RenderBeastTestSuite : public TestSuite
{
public:
RenderBeastTestSuite();

private:
void testTextureRowAllocator();
};

} // namespace bs
11 changes: 11 additions & 0 deletions Source/Plugins/bsfRenderBeast/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,4 +25,15 @@ if(RENDERER_MODULE MATCHES "RenderBeast")
install_bsf_target(bsfRenderBeast)
endif()

if (BUILD_TESTS)

add_executable(bsfRenderBeastTest
BsRenderBeastTest.cpp)
target_include_directories(bsfRenderBeastTest PRIVATE "./")
target_link_libraries(bsfRenderBeastTest bsf bsfRenderBeast)

set_property(TARGET bsfRenderBeastTest PROPERTY FOLDER Tests)
bs_add_test(NAME RenderBeastTests COMMAND $<TARGET_FILE:bsfRenderBeastTest>)
endif()

conditional_cotire(bsfRenderBeast)