Add a unit test suite (doctest, MSVC Win32) and run it in CI

.github/workflows/build.yml

@@ -2,7 +2,8 @@ name: Build
 
 # Builds BH.dll (Release|Win32) with MSVC on every pull request and push to main,
 # so a change that breaks the build is caught before it is merged. The built DLL is
-# attached to each run as a downloadable artifact for testing.
+# attached to each run as a downloadable artifact for testing. A second job builds
+# and runs the unit tests (tests/BH.Tests.vcxproj); any failing test fails the run.
 
 on:
   pull_request:
@@ -43,3 +44,35 @@ jobs:
             **/Release/BH.dll
             **/Release/BH.pdb
           if-no-files-found: error
+
+  tests:
+    name: Unit tests (MSVC Release|Win32)
+    runs-on: windows-latest
+    steps:
+      - uses: actions/checkout@v4
+
+      - uses: microsoft/setup-msbuild@v2
+
+      - name: Build tests
+        run: msbuild tests/BH.Tests.vcxproj /m /p:Configuration=Release /p:Platform=Win32
+
+      - name: Run tests
+        working-directory: tests
+        run: bin\Release\BH.Tests.exe --duration=false
+
+      - name: Write JUnit report
+        if: always()
+        working-directory: tests
+        # Same tests again, written as JUnit XML for the artifact. Pass/fail is decided by
+        # the previous step, so this one never fails on its own.
+        run: |
+          bin\Release\BH.Tests.exe --reporters=junit --out=junit.xml
+          exit 0
+
+      - name: Upload JUnit report
+        if: always()
+        uses: actions/upload-artifact@v4
+        with:
+          name: test-results-${{ github.sha }}
+          path: tests/junit.xml
+          if-no-files-found: ignore

.gitignore

@@ -39,3 +39,8 @@ Release/*
 /BH.VC.db
 /.vs
 out/
+
+# Unit test build output
+tests/bin/
+tests/obj/
+tests/junit.xml

README.md

@@ -18,3 +18,14 @@ You can change the command line arguments as you see fit.
 ```
 
 Any debug or changed version will only work in *Single Player*. Do not enter multiplayer with a modified BH
+
+## Unit tests
+
+`tests/BH.Tests.vcxproj` builds a console program that runs BH's game-independent logic (loot filter rules, formulas, config, stash export templates and more) against small fakes of the game functions it calls. It is not part of `BH.sln`, so building BH is unchanged. CI builds and runs it on every pull request.
+
+```
+msbuild tests\BH.Tests.vcxproj /p:Configuration=Release /p:Platform=Win32
+tests\bin\Release\BH.Tests.exe
+```
+
+Or open `tests\BH.Tests.vcxproj` in Visual Studio and run it. Run `BH.Tests.exe --help` for filtering options (for example `-ts=LootFilterRules` runs one suite). Tests marked `should_fail` document known bugs: they fail today, and the run turns red once the bug is fixed so the marker can be removed.

ThirdParty/doctest/LICENSE.txt

@@ -0,0 +1,21 @@
+The MIT License (MIT)
+
+Copyright (c) 2016-2023 Viktor Kirilov
+
+Permission is hereby granted, free of charge, to any person obtaining a copy
+of this software and associated documentation files (the "Software"), to deal
+in the Software without restriction, including without limitation the rights
+to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+copies of the Software, and to permit persons to whom the Software is
+furnished to do so, subject to the following conditions:
+
+The above copyright notice and this permission notice shall be included in all
+copies or substantial portions of the Software.
+
+THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+SOFTWARE.

tests/AsyncDrawBufferTests.cpp

@@ -0,0 +1,125 @@
+// AsyncDrawBuffer / DrawDirective (AsyncDrawBuffer.cpp): the double-buffered draw list the automap
+// item markers are drawn from (MapNotify: a synchronous DrawDirective with maxGhost 5, whose director
+// pushes one top-layer draw call per marker). A frame is built in the back buffer and shown only after
+// a swap; the directive rebuilds its frame only every few game frames (redrawing the last one in
+// between, at most `maxGhost` times), or straight away after forceUpdate() (MapNotify forces one when
+// the player changes act).
+#include <algorithm>
+#include <functional>
+#include <string>
+#include <vector>
+
+#include "doctest/doctest.h"
+
+#include "AsyncDrawBuffer.h"
+
+namespace {
+
+// Draw calls that record what they drew.
+struct Canvas {
+	std::vector<std::string> drawn;
+	std::function<void()> Draw(const std::string& what) {
+		return [this, what]() { drawn.push_back(what); };
+	}
+};
+
+// Runs `frames` game frames of a synchronous DrawDirective whose director builds frame number
+// 1, 2, 3... each time it runs. Returns what was on screen each frame ("" = nothing).
+std::vector<std::string> Frames(DrawDirective& directive, int frames, std::vector<int> forceBeforeFrame = {}) {
+	Canvas canvas;
+	int built = 0;
+	std::vector<std::string> shown;
+	for (int frame = 1; frame <= frames; frame++) {
+		if (std::find(forceBeforeFrame.begin(), forceBeforeFrame.end(), frame) != forceBeforeFrame.end()) {
+			directive.forceUpdate();
+		}
+		canvas.drawn.clear();
+		directive.draw([&](AsyncDrawBuffer& buffer) {
+			built++;
+			buffer.push_top_layer(canvas.Draw(std::to_string(built)));
+		});
+		shown.push_back(canvas.drawn.empty() ? "" : canvas.drawn[0]);
+	}
+	return shown;
+}
+
+std::vector<std::string> S(std::initializer_list<const char*> values) {
+	return std::vector<std::string>(values.begin(), values.end());
+}
+
+}  // namespace
+
+TEST_SUITE("AsyncDrawBuffer") {
+	TEST_CASE("draw calls appear only after the buffers are swapped, in the order they were pushed") {
+		Canvas canvas;
+		AsyncDrawBuffer buffer;
+		buffer.push_top_layer(canvas.Draw("a"));
+		buffer.push_top_layer(canvas.Draw("b"));
+		buffer.drawAll();
+		CHECK(canvas.drawn.empty());
+
+		buffer.swapBuffers();
+		buffer.drawAll();
+		CHECK(canvas.drawn == S({ "a", "b" }));
+	}
+
+	TEST_CASE("the shown frame is drawn again every time until the next swap") {
+		Canvas canvas;
+		AsyncDrawBuffer buffer;
+		buffer.push_top_layer(canvas.Draw("a"));
+		buffer.swapBuffers();
+		buffer.push_top_layer(canvas.Draw("b"));  // the next frame, still being built
+		buffer.drawAll();
+		buffer.drawAll();
+		CHECK(canvas.drawn == S({ "a", "a" }));
+	}
+
+	TEST_CASE("clear discards the frame being built, not the one shown") {
+		Canvas canvas;
+		AsyncDrawBuffer buffer;
+		buffer.push_top_layer(canvas.Draw("shown"));
+		buffer.swapBuffers();
+		buffer.push_top_layer(canvas.Draw("discarded"));
+		buffer.clear();
+		buffer.drawAll();
+		CHECK(canvas.drawn == S({ "shown" }));
+
+		canvas.drawn.clear();
+		buffer.swapBuffers();
+		buffer.drawAll();
+		CHECK(canvas.drawn.empty());
+	}
+
+	TEST_CASE("each rebuild replaces the previous frame instead of adding to it") {
+		DrawDirective directive(true, 5);
+		Canvas canvas;
+		for (int frame = 0; frame < 3; frame++) {
+			canvas.drawn.clear();
+			directive.forceUpdate();
+			directive.draw([&](AsyncDrawBuffer& buffer) {
+				buffer.push_top_layer(canvas.Draw("marker"));
+			});
+		}
+		CHECK(canvas.drawn == S({ "marker" }));
+	}
+
+	TEST_CASE("the automap directive rebuilds its frame every 6 frames and redraws it in between") {
+		DrawDirective directive(true, 5);  // as MapNotify's automap markers
+		std::vector<std::string> shown = Frames(directive, 25);
+		// The first frame is built within maxGhost + 2 frames...
+		auto first = std::find(shown.begin(), shown.end(), "1");
+		REQUIRE(first != shown.end());
+		CHECK(first - shown.begin() <= 6);
+		// ...and from then on each frame is shown 6 times (built once, redrawn maxGhost times).
+		REQUIRE(shown.end() - first >= 18);
+		std::vector<std::string> steady(first, first + 18);
+		CHECK(steady == S({ "1", "1", "1", "1", "1", "1", "2", "2", "2", "2", "2", "2", "3", "3", "3", "3", "3", "3" }));
+	}
+
+	TEST_CASE("forceUpdate rebuilds on the very next frame") {
+		DrawDirective directive(true, 5);
+		// Force the first frame and again on frame 3, well before the 6-frame refresh.
+		std::vector<std::string> shown = Frames(directive, 5, { 1, 3 });
+		CHECK(shown == S({ "1", "1", "2", "2", "2" }));
+	}
+}

tests/BH.Tests.vcxproj

@@ -0,0 +1,141 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!--
+  BH.Tests: unit tests for BH's game-independent logic (loot filter rules and formulas, config
+  files, stash export templates, game list filter, stats panel tables, automap info, party and
+  chat handling, UI controls). Builds a console exe that compiles the BH sources under test
+  directly, plus small fakes for the game functions they call (tests/fakes).
+
+  Not part of BH.sln on purpose, so building BH is unchanged. To run the tests:
+    msbuild tests\BH.Tests.vcxproj /p:Configuration=Release /p:Platform=Win32
+    tests\bin\Release\BH.Tests.exe
+  or open this project in Visual Studio and run it (Ctrl+F5).
+-->
+<Project DefaultTargets="Build" ToolsVersion="12.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
+  <ItemGroup Label="ProjectConfigurations">
+    <ProjectConfiguration Include="Debug|Win32">
+      <Configuration>Debug</Configuration>
+      <Platform>Win32</Platform>
+    </ProjectConfiguration>
+    <ProjectConfiguration Include="Release|Win32">
+      <Configuration>Release</Configuration>
+      <Platform>Win32</Platform>
+    </ProjectConfiguration>
+  </ItemGroup>
+  <PropertyGroup Label="Globals">
+    <ProjectGuid>{7D3F1C2A-5B8E-4C61-9A0D-2E4B6F8A1C35}</ProjectGuid>
+    <RootNamespace>BHTests</RootNamespace>
+  </PropertyGroup>
+  <Import Project="$(VCTargetsPath)\Microsoft.Cpp.Default.props" />
+  <PropertyGroup Label="Configuration">
+    <ConfigurationType>Application</ConfigurationType>
+    <CharacterSet>MultiByte</CharacterSet>
+    <PlatformToolset Condition="'$(VisualStudioVersion)' == '15.0'">v141</PlatformToolset>
+    <PlatformToolset Condition="'$(VisualStudioVersion)' == '16.0'">v142</PlatformToolset>
+    <PlatformToolset Condition="'$(VisualStudioVersion)' == '17.0'">v143</PlatformToolset>
+    <PlatformToolset Condition="'$(VisualStudioVersion)' == '18.0'">v143</PlatformToolset>
+  </PropertyGroup>
+  <PropertyGroup Condition="'$(Configuration)'=='Debug'" Label="Configuration">
+    <UseDebugLibraries>true</UseDebugLibraries>
+  </PropertyGroup>
+  <PropertyGroup Condition="'$(Configuration)'=='Release'" Label="Configuration">
+    <UseDebugLibraries>false</UseDebugLibraries>
+  </PropertyGroup>
+  <Import Project="$(VCTargetsPath)\Microsoft.Cpp.props" />
+  <PropertyGroup>
+    <OutDir>$(ProjectDir)bin\$(Configuration)\</OutDir>
+    <IntDir>$(ProjectDir)obj\$(Configuration)\</IntDir>
+    <TargetName>BH.Tests</TargetName>
+    <LocalDebuggerWorkingDirectory>$(ProjectDir)</LocalDebuggerWorkingDirectory>
+  </PropertyGroup>
+  <ItemDefinitionGroup>
+    <ClCompile>
+      <WarningLevel>Level3</WarningLevel>
+      <!-- Same include layout and source charset as BH.vcxproj. -->
+      <AdditionalIncludeDirectories>..\BH;..\ThirdParty;..\ThirdParty\cpp-lru-cache\include;fakes;support;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
+      <AdditionalOptions>/utf-8 /bigobj %(AdditionalOptions)</AdditionalOptions>
+    </ClCompile>
+    <Link>
+      <SubSystem>Console</SubSystem>
+      <GenerateDebugInformation>true</GenerateDebugInformation>
+      <AdditionalDependencies>shlwapi.lib;version.lib;%(AdditionalDependencies)</AdditionalDependencies>
+    </Link>
+  </ItemDefinitionGroup>
+  <ItemDefinitionGroup Condition="'$(Configuration)'=='Debug'">
+    <ClCompile>
+      <Optimization>Disabled</Optimization>
+      <RuntimeLibrary>MultiThreadedDebug</RuntimeLibrary>
+    </ClCompile>
+  </ItemDefinitionGroup>
+  <ItemDefinitionGroup Condition="'$(Configuration)'=='Release'">
+    <ClCompile>
+      <Optimization>MaxSpeed</Optimization>
+      <RuntimeLibrary>MultiThreaded</RuntimeLibrary>
+    </ClCompile>
+  </ItemDefinitionGroup>
+  <!-- BH sources under test, compiled unmodified. -->
+  <ItemGroup>
+    <ClCompile Include="..\BH\AsyncDrawBuffer.cpp" />
+    <ClCompile Include="..\BH\Common.cpp" />
+    <ClCompile Include="..\BH\Config.cpp" />
+    <ClCompile Include="..\BH\D2Helpers.cpp" />
+    <ClCompile Include="..\BH\Drawing\Advanced\Checkhook\Checkhook.cpp" />
+    <ClCompile Include="..\BH\Drawing\Advanced\Combohook\Combohook.cpp" />
+    <ClCompile Include="..\BH\Drawing\Advanced\Keyhook\Keyhook.cpp" />
+    <ClCompile Include="..\BH\Drawing\Stats\StatsDisplay.cpp" />
+    <ClCompile Include="..\BH\JSONObject.cpp" />
+    <ClCompile Include="..\BH\Modules\Bnet\Bnet.cpp" />
+    <ClCompile Include="..\BH\Modules\ChatColor\ChatColor.cpp" />
+    <ClCompile Include="..\BH\Modules\Gamefilter\Gamefilter.cpp" />
+    <ClCompile Include="..\BH\Modules\Gamefilter\ParsedFilterString.cpp" />
+    <ClCompile Include="..\BH\Modules\Item\ItemDisplay.cpp" />
+    <ClCompile Include="..\BH\Modules\MapNotify\MapNotify.cpp" />
+    <ClCompile Include="..\BH\Modules\Party\Party.cpp" />
+    <ClCompile Include="..\BH\Modules\ScreenInfo\ScreenInfo.cpp" />
+    <ClCompile Include="..\BH\Mustache.cpp" />
+  </ItemGroup>
+  <!-- BH sources the code under test links against (drawing hooks, task queue). -->
+  <ItemGroup>
+    <ClCompile Include="..\BH\Drawing\Advanced\Colorhook\Colorhook.cpp" />
+    <ClCompile Include="..\BH\Drawing\Basic\Boxhook\Boxhook.cpp" />
+    <ClCompile Include="..\BH\Drawing\Basic\Crosshook\Crosshook.cpp" />
+    <ClCompile Include="..\BH\Drawing\Basic\Framehook\Framehook.cpp" />
+    <ClCompile Include="..\BH\Drawing\Basic\Texthook\Texthook.cpp" />
+    <ClCompile Include="..\BH\Drawing\Hook.cpp" />
+    <ClCompile Include="..\BH\Task.cpp" />
+  </ItemGroup>
+  <!-- Stand-ins for the game and for BH globals defined in files that are not compiled here. -->
+  <ItemGroup>
+    <ClCompile Include="fakes\BHGlobals.cpp" />
+    <ClCompile Include="fakes\EnginePtrs.cpp" />
+    <ClCompile Include="fakes\FakeEngine.cpp" />
+  </ItemGroup>
+  <!-- Shared test helpers. -->
+  <ItemGroup>
+    <ClCompile Include="support\LootFilter.cpp" />
+  </ItemGroup>
+  <ItemGroup>
+    <ClCompile Include="main.cpp" />
+    <ClCompile Include="AsyncDrawBufferTests.cpp" />
+    <ClCompile Include="BnetTests.cpp" />
+    <ClCompile Include="ChatColorTests.cpp" />
+    <ClCompile Include="CheckhookTests.cpp" />
+    <ClCompile Include="CombohookTests.cpp" />
+    <ClCompile Include="CommonTests.cpp" />
+    <ClCompile Include="ConfigTests.cpp" />
+    <ClCompile Include="D2HelpersTests.cpp" />
+    <ClCompile Include="FormulaTests.cpp" />
+    <ClCompile Include="GameFilterTests.cpp" />
+    <ClCompile Include="ItemConditionTests.cpp" />
+    <ClCompile Include="ItemStatConditionTests.cpp" />
+    <ClCompile Include="JSONObjectTests.cpp" />
+    <ClCompile Include="KeyhookTests.cpp" />
+    <ClCompile Include="LootFilterActionTests.cpp" />
+    <ClCompile Include="LootFilterRulesTests.cpp" />
+    <ClCompile Include="MapNotifyTests.cpp" />
+    <ClCompile Include="MustacheTests.cpp" />
+    <ClCompile Include="PartyTests.cpp" />
+    <ClCompile Include="ScreenInfoTests.cpp" />
+    <ClCompile Include="StatsDisplayTests.cpp" />
+  </ItemGroup>
+  <Import Project="$(VCTargetsPath)\Microsoft.Cpp.targets" />
+</Project>