From 797bdcd97c7ca56ecaa31eebbc13a0f2b31c4eee Mon Sep 17 00:00:00 2001 From: black-sliver <59490463+black-sliver@users.noreply.github.com> Date: Wed, 8 May 2024 09:48:48 +0200 Subject: [PATCH] Test: add unit testing and some tests * jsonutil stringsplit * location/section parser inheritance for empty child rules --- .github/workflows/binaries.yaml | 7 ++-- .github/workflows/release.yaml | 6 ++-- .github/workflows/test.yaml | 5 +-- Makefile | 43 ++++++++++++++++++++---- test/README.md | 38 +++++++++++++++++++++ test/core/test_locations_parser.cpp | 51 +++++++++++++++++++++++++++++ test/jsonutil/test_commasplit.cpp | 40 ++++++++++++++++++++++ 7 files changed, 175 insertions(+), 15 deletions(-) create mode 100644 test/README.md create mode 100644 test/core/test_locations_parser.cpp create mode 100644 test/jsonutil/test_commasplit.cpp diff --git a/.github/workflows/binaries.yaml b/.github/workflows/binaries.yaml index f7aaabe..37610eb 100644 --- a/.github/workflows/binaries.yaml +++ b/.github/workflows/binaries.yaml @@ -21,7 +21,7 @@ jobs: - name: Install dependencies run: | sudo apt-get update -y -qq - sudo apt-get install coreutils build-essential libsdl2-dev libsdl2-image-dev libsdl2-ttf-dev p7zip + sudo apt-get install coreutils build-essential libsdl2-dev libsdl2-image-dev libsdl2-ttf-dev p7zip libgtest-dev - uses: actions/checkout@v4 with: submodules: recursive @@ -48,7 +48,7 @@ jobs: - name: Install dependencies run: | sudo apt-get update -y -qq - sudo apt-get install coreutils build-essential libsdl2-dev libsdl2-image-dev libsdl2-ttf-dev p7zip wget + sudo apt-get install coreutils build-essential libsdl2-dev libsdl2-image-dev libsdl2-ttf-dev p7zip wget libgtest-dev - uses: actions/checkout@v4 with: submodules: recursive @@ -74,7 +74,7 @@ jobs: steps: - name: Install dependencies run: | - brew install coreutils SDL2 sdl2_ttf sdl2_image openssl@1.1 automake libtool autoconf + brew install coreutils SDL2 sdl2_ttf sdl2_image openssl@1.1 automake libtool autoconf googletest - uses: actions/checkout@v4 with: submodules: recursive @@ -123,6 +123,7 @@ jobs: mingw64/mingw-w64-x86_64-SDL2_ttf mingw64/mingw-w64-x86_64-freetype mingw64/mingw-w64-x86_64-openssl + mingw64/mingw-w64-x86_64-gtest p7zip - uses: actions/checkout@v4 with: diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index c3238ef..fd01d4c 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -41,7 +41,7 @@ jobs: - name: Install dependencies run: | sudo apt-get update -y -qq - sudo apt-get install coreutils build-essential libsdl2-dev libsdl2-image-dev libsdl2-ttf-dev p7zip wget + sudo apt-get install coreutils build-essential libsdl2-dev libsdl2-image-dev libsdl2-ttf-dev p7zip wget libgtest-dev - uses: actions/checkout@v4 with: submodules: recursive @@ -77,7 +77,7 @@ jobs: - name: Install dependencies run: | sudo apt-get update -y -qq - sudo apt-get install coreutils build-essential libsdl2-dev libsdl2-image-dev libsdl2-ttf-dev p7zip wget + sudo apt-get install coreutils build-essential libsdl2-dev libsdl2-image-dev libsdl2-ttf-dev p7zip wget libgtest-dev - uses: actions/checkout@v4 with: submodules: recursive @@ -106,7 +106,7 @@ jobs: steps: - name: Install dependencies run: | - brew install coreutils SDL2 sdl2_ttf sdl2_image openssl@1.1 automake libtool autoconf + brew install coreutils SDL2 sdl2_ttf sdl2_image openssl@1.1 automake libtool autoconf googletest - uses: actions/checkout@v4 with: submodules: recursive diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index e6bde3c..2caf117 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -47,11 +47,11 @@ jobs: if: ${{ startsWith(matrix.os, 'ubuntu') }} run: | sudo apt-get update -y -qq - sudo apt-get install coreutils build-essential libsdl2-dev libsdl2-image-dev libsdl2-ttf-dev + sudo apt-get install coreutils build-essential libsdl2-dev libsdl2-image-dev libsdl2-ttf-dev libgtest-dev - name: Install dependencies (brew) if: ${{ startsWith(matrix.os, 'macos') }} run: | - brew install coreutils SDL2 sdl2_ttf sdl2_image openssl@1.1 + brew install coreutils SDL2 sdl2_ttf sdl2_image openssl@1.1 googletest - name: Install dependencies (msys2) if: ${{ startsWith(matrix.os, 'windows') }} uses: msys2/setup-msys2@v2 @@ -68,6 +68,7 @@ jobs: mingw64/mingw-w64-x86_64-SDL2_ttf mingw64/mingw-w64-x86_64-freetype mingw64/mingw-w64-x86_64-openssl + mingw64/mingw-w64-x86_64-gtest p7zip - uses: actions/checkout@v4 with: diff --git a/Makefile b/Makefile index e1eaf75..95e3153 100644 --- a/Makefile +++ b/Makefile @@ -3,6 +3,7 @@ # for now we use SDL2_image GIF support only CONF ?= RELEASE # make CONF=DEBUG for debug, CONF=DIST for .zip SRC_DIR = src +TEST_DIR = test LIB_DIR = lib SRC = $(wildcard $(SRC_DIR)/*.cpp) \ $(wildcard $(SRC_DIR)/uilib/*.cpp) \ @@ -28,6 +29,8 @@ HDR = $(wildcard $(SRC_DIR)/*.h) \ $(wildcard $(SRC_DIR)/http/*.h) \ $(wildcard $(SRC_DIR)/packmanager/*.h) \ $(wildcard $(LIB_DIR)/tinyfiledialogs/*) +TEST_SRC = $(filter-out $(SRC_DIR)/main.cpp,$(SRC)) \ + $(wildcard $(TEST_DIR)/*/*.cpp) INCLUDE_DIRS = -Ilib -Ilib/lua -Ilib/asio/include -DASIO_STANDALONE -Ilib/miniz -Ilib/json/include -Ilib/valijson/include -Ilib/tinyfiledialogs -Ilib/wswrap/include #-Ilib/gifdec WIN32_INCLUDE_DIRS = -Iwin32-lib/i686/include WIN32_LIB_DIRS = -L./win32-lib/i686/bin -L./win32-lib/i686/lib @@ -74,13 +77,17 @@ UNAME = $(shell uname -sm | tr -s ' ' '-' | tr A-Z a-z ) DIST_DIR ?= dist BUILD_DIR ?= build EXE_NAME = poptracker +TEST_EXE_NAME = poptracker-test NIX_BUILD_DIR = $(BUILD_DIR)/$(UNAME) WIN32_BUILD_DIR = $(BUILD_DIR)/win32 WIN64_BUILD_DIR = $(BUILD_DIR)/win64 WASM_BUILD_DIR = $(BUILD_DIR)/wasm WIN32_EXE = $(WIN32_BUILD_DIR)/$(EXE_NAME).exe +WIN32_TEST_EXE = $(WIN32_BUILD_DIR)/$(TEST_EXE_NAME).exe WIN64_EXE = $(WIN64_BUILD_DIR)/$(EXE_NAME).exe +WIN64_TEST_EXE = $(WIN64_BUILD_DIR)/$(TEST_EXE_NAME).exe NIX_EXE = $(NIX_BUILD_DIR)/$(EXE_NAME) +NIX_TEST_EXE = $(NIX_BUILD_DIR)/$(TEST_EXE_NAME) HTML = $(WASM_BUILD_DIR)/$(EXE_NAME).html # dist/zip @@ -99,11 +106,14 @@ endif # fragments NIX_OBJ := $(patsubst %.cpp, $(NIX_BUILD_DIR)/%.o, $(SRC)) -NIX_OBJ_DIRS := $(sort $(dir $(NIX_OBJ))) +NIX_TEST_OBJ := $(patsubst %.cpp, $(NIX_BUILD_DIR)/%.o, $(TEST_SRC)) +NIX_OBJ_DIRS := $(sort $(dir $(NIX_OBJ)) $(dir $(NIX_TEST_OBJ))) WIN32_OBJ := $(patsubst %.cpp, $(WIN32_BUILD_DIR)/%.o, $(SRC)) -WIN32_OBJ_DIRS := $(sort $(dir $(WIN32_OBJ))) +WIN32_TEST_OBJ := $(patsubst %.cpp, $(WIN32_BUILD_DIR)/%.o, $(TEST_SRC)) +WIN32_OBJ_DIRS := $(sort $(dir $(WIN32_OBJ)) $(dir $(WIN32_TEST_OBJ))) WIN64_OBJ := $(patsubst %.cpp, $(WIN64_BUILD_DIR)/%.o, $(SRC)) -WIN64_OBJ_DIRS := $(sort $(dir $(WIN64_OBJ))) +WIN64_TEST_OBJ := $(patsubst %.cpp, $(WIN64_BUILD_DIR)/%.o, $(TEST_SRC)) +WIN64_OBJ_DIRS := $(sort $(dir $(WIN64_OBJ)) $(dir $(WIN64_TEST_OBJ))) # tools CC = gcc # TODO: use ?= @@ -179,6 +189,7 @@ endif # default target: "native" ifdef IS_WIN32 EXE = $(WIN32_EXE) + TEST_EXE = $(WIN32_TEST_EXE) WIN32CC = $(CC) WIN32CPP = $(CPP) WIN32AR = $(AR) @@ -193,6 +204,7 @@ ifdef IS_WIN32 endif else ifdef IS_WIN64 EXE = $(WIN64_EXE) + TEST_EXE = $(WIN64_TEST_EXE) WIN64CC = $(CC) WIN64CPP = $(CPP) WIN64AR = $(AR) @@ -207,6 +219,7 @@ else ifdef IS_WIN64 endif else ifdef IS_OSX EXE = $(NIX_EXE) + TEST_EXE = $(NIX_TEST_EXE) ifeq ($(CONF), DIST) # TODO dmg? native: $(OSX_APP) $(OSX_ZIP) test_osx_app else @@ -216,6 +229,7 @@ else WIN32_LIBS += -lbrotlidec-static -lbrotlicommon-static # brotli is a mess WIN64_LIBS += -lbrotlidec-static -lbrotlicommon-static EXE = $(NIX_EXE) + TEST_EXE = $(NIX_TEST_EXE) ifeq ($(CONF), DIST) # TODO deb? native: $(NIX_XZ) else @@ -232,6 +246,8 @@ cross: $(WIN32_ZIP) $(WIN64_ZIP) else cross: $(WIN32_EXE) $(WIN64_EXE) endif +cross-test: $(WIN32_TEST_EXE) $(WIN64_TEST_EXE) + # TODO: run tests with wine # Project Targets $(HTML): $(SRC) $(WASM_BUILD_DIR)/liblua.a $(HDR) | $(WASM_BUILD_DIR) @@ -243,6 +259,9 @@ $(HTML): $(SRC) $(WASM_BUILD_DIR)/liblua.a $(HDR) | $(WASM_BUILD_DIR) $(NIX_EXE): $(NIX_OBJ) $(NIX_BUILD_DIR)/liblua.a $(HDR) | $(NIX_BUILD_DIR) $(CPP) -std=c++1z $(NIX_OBJ) $(NIX_BUILD_DIR)/liblua.a -ldl $(NIX_LD_FLAGS) `sdl2-config --libs` $(NIX_LIBS) -o $@ +$(NIX_TEST_EXE): $(NIX_TEST_OBJ) $(NIX_BUILD_DIR)/liblua.a $(HDR) | $(NIX_BUILD_DIR) + $(CPP) -std=c++1z $(NIX_TEST_OBJ) -l gtest -l gtest_main $(NIX_BUILD_DIR)/liblua.a -ldl $(NIX_LD_FLAGS) `sdl2-config --libs` $(NIX_LIBS) -o $@ + $(WIN32_EXE): $(WIN32_OBJ) $(WIN32_BUILD_DIR)/app.res $(WIN32_BUILD_DIR)/liblua.a $(HDR) | $(WIN32_BUILD_DIR) # FIXME: static 32bit exe does not work for some reason $(WIN32CPP) -o $@ -std=c++17 -static -Wl,-Bstatic $(WIN32_OBJ) $(WIN32_BUILD_DIR)/app.res $(WIN32_BUILD_DIR)/liblua.a $(WIN32_LIB_DIRS) $(WIN32_LD_FLAGS) $(WIN32_LIBS) @@ -250,12 +269,18 @@ ifneq ($(CONF), DEBUG) $(WIN32STRIP) $@ endif +$(WIN32_TEST_EXE): $(WIN32_TEST_OBJ) $(WIN32_BUILD_DIR)/app.res $(WIN32_BUILD_DIR)/liblua.a $(HDR) | $(WIN32_BUILD_DIR) + $(WIN32CPP) -o $@ -std=c++17 $(WIN32_TEST_OBJ) -l gtest -l gtest_main $(WIN32_BUILD_DIR)/liblua.a $(WIN32_LIB_DIRS) $(WIN32_LD_FLAGS) $(WIN32_LIBS) + $(WIN64_EXE): $(WIN64_OBJ) $(WIN64_BUILD_DIR)/app.res $(WIN64_BUILD_DIR)/liblua.a $(HDR) | $(WIN64_BUILD_DIR) $(WIN64CPP) -o $@ -std=c++17 -static -Wl,-Bstatic $(WIN64_OBJ) $(WIN64_BUILD_DIR)/app.res $(WIN64_BUILD_DIR)/liblua.a $(WIN64_LIB_DIRS) $(WIN64_LD_FLAGS) $(WIN64_LIBS) ifneq ($(CONF), DEBUG) $(WIN64STRIP) $@ endif +$(WIN64_TEST_EXE): $(WIN64_TEST_OBJ) $(WIN64_BUILD_DIR)/app.res $(WIN64_BUILD_DIR)/liblua.a $(HDR) | $(WIN64_BUILD_DIR) + $(WIN64CPP) -o $@ -std=c++17 $(WIN64_TEST_OBJ) -l gtest -l gtest_main $(WIN64_BUILD_DIR)/liblua.a $(WIN64_LIB_DIRS) $(WIN64_LD_FLAGS) $(WIN64_LIBS) + $(WIN32_ZIP): $(WIN32_EXE) | $(DIST_DIR) $(WIN64_ZIP): $(WIN64_EXE) | $(DIST_DIR) $(WIN32_ZIP) $(WIN64_ZIP): @@ -369,12 +394,16 @@ $(WIN64_BUILD_DIR)/%.o: %.c* $(HDR) | $(WIN64_OBJ_DIRS) # Avoid detection/auto-cleanup of intermediates .OBJ_DIRS: $(NIX_OBJ_DIRS) $(WIN32_OBJ_DIRS) $(WIN64_OBJ_DIRS) -test: $(EXE) -# TODO: implement and run actual tests - @echo "Testing $(EXE)" +test: $(EXE) ${TEST_EXE} + @echo "Running $(TEST_EXE)" + @$(TEST_EXE) + @echo "Checking $(EXE)" + @echo -n "Size: " @du -h $(EXE) | cut -f -1 + @echo -n "Version: " @timeout 5 $(EXE) --version - @timeout 9 $(EXE) --list-packs + @echo "HTTP Test ..." + @timeout 9 $(EXE) --list-packs > /dev/null clean: (cd lib/lua && make -f makefile clean) diff --git a/test/README.md b/test/README.md new file mode 100644 index 0000000..cfa2e55 --- /dev/null +++ b/test/README.md @@ -0,0 +1,38 @@ +# Automated PopTracker Tests + +PRs to add tests are welcome. + +We have 3 types of tests in mind: +- **Unit tests:** test individual code fragments using googletest, see [How to add Unit Tests](#how-to-add-unit-tests). +- **Render tests:** load packs and verify render output using `SDL_VIDEODRIVER=dummy` and a screenshot +- **High level tests:** stuff that should be tested end-to-end may be hard to do with googletest because we'd need to + mock all of PopTracker. Those will probably use an external test script that waits for e.g. a specific print from Lua. + +Tests should be run on both 64bit and 32bit to see if any integer size assumptions are wrong. + +Tests that should be done: +- Render tests for all example packs +- High level tests for AP + - 64bit onItem + - 64bit onLocationScout + - 64bit onLocationCheck + - 64bit CheckedLocations + - 64bit MissingLocations + - 64bit data storage + - unicode data storage + - 64bit slot data + - unicode slot data + +## How to add Unit Tests + +We compile `test/*/*.cpp` together with googletest and `src/**` into a test binary. To add tests, simply pick a folder +and add a `test_*.cpp` file that uses googletest API. + +## How to run Unit Tests + +* install `googltest`, sometimes called `gtest` or `libgtest-dev`, via pacman, apt, brew, etc. or from source +* `make test` + +## Other Tests + +We have not decided on a solution for non unit tests yet. diff --git a/test/core/test_locations_parser.cpp b/test/core/test_locations_parser.cpp new file mode 100644 index 0000000..cd56399 --- /dev/null +++ b/test/core/test_locations_parser.cpp @@ -0,0 +1,51 @@ +#include +#include +#include "../../src/core/location.h" + + +using namespace std; +using nlohmann::json; + + +TEST(LocationsParserTest, LocationRulesInheritAndEmpty) { + // TODO: validate indirectly by checcking accessibility/visibilty via Tracker + const list> parentAccessRules = {{"a"}}; + const list> parentVisibilityRules = {{"b"}}; + json childNode = R"( + { + "name": "child", + "access_rules": [], + "visibility_rules": [] + } + )"_json; + + auto location = Location::FromJSON(childNode, {}, parentAccessRules, parentVisibilityRules).front(); + EXPECT_EQ(location.getAccessRules(), + parentAccessRules); + EXPECT_EQ(location.getVisibilityRules(), + parentVisibilityRules); +} + +TEST(LocationsParserTest, SectionRulesInheritAndEmpty) { + // TODO: validate indirectly by checcking accessibility/visibilty via Tracker + json locationNode = R"( + { + "name": "location", + "access_rules": [["a"]], + "visibility_rules": [["b"]], + "sections": [ + { + "name": "section", + "access_rules": [], + "visibility_rules": [] + } + ] + } + )"_json; + + auto section = Location::FromJSON(locationNode, {}, {}, {}).front().getSections().front(); + EXPECT_EQ(section.getAccessRules(), + list>({{"a"}})); + EXPECT_EQ(section.getVisibilityRules(), + list>({{"b"}})); +} diff --git a/test/jsonutil/test_commasplit.cpp b/test/jsonutil/test_commasplit.cpp new file mode 100644 index 0000000..4b8392d --- /dev/null +++ b/test/jsonutil/test_commasplit.cpp @@ -0,0 +1,40 @@ +#include +#include +#include +#include "../../src/core/jsonutil.h" + + +using namespace std; + + +// Living standard :TM: +// only trailing empty fields should be dropped + +TEST(CommaSplitTest, Empty) { + EXPECT_EQ(commasplit(""), + list({})); +} + +TEST(CommaSplitTest, TrailingComma) { + // Trailing empty values should be dropped + EXPECT_EQ(commasplit("a,"), + list({"a"})); +} + +TEST(CommaSplitTest, LeadingEmpty) { + // Leading empty values should be kept + EXPECT_EQ(commasplit(",b"), + list({"", "b"})); +} + +TEST(CommaSplitTest, MiddleEmpty) { + // Empty values in the middle should be kept + EXPECT_EQ(commasplit("a,,b"), + list({"a", "", "b"})); +} + +TEST(CommaSplitTest, TrailingWhitespace) { + // If trailing whitespace results in trailing comma, it should behave the same as TrailingComma + EXPECT_EQ(commasplit("a, "), + list({"a"})); +}