From 16de4b82d57f1c0de6eb40f37143c32d4da36a02 Mon Sep 17 00:00:00 2001 From: LocalIdentity Date: Tue, 8 Sep 2026 10:30:30 +1000 Subject: [PATCH 01/14] Update Spec --- src/Export/spec.lua | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/Export/spec.lua b/src/Export/spec.lua index 4b947a5b528..696a2d8d2dc 100644 --- a/src/Export/spec.lua +++ b/src/Export/spec.lua @@ -13333,17 +13333,17 @@ return { }, [3]={ list=false, - name="", + name="GameObjectRegisterKey", refTo="", type="Int", - width=80 + width=170 }, [4]={ list=false, - name="", - refTo="", + name="SummonEffect", + refTo="MiscAnimated", type="Key", - width=80 + width=170 }, [5]={ list=false, @@ -13382,15 +13382,15 @@ return { }, [10]={ list=false, - name="", - refTo="", + name="BuffDefinition", + refTo="BuffDefinitions", type="Key", width=80 }, [11]={ list=false, - name="", - refTo="", + name="BuffVisual", + refTo="BuffVisuals", type="Key", width=80 }, From 687405e473599c7da1c2995d2a69db1265800a9a Mon Sep 17 00:00:00 2001 From: AdamZ-8113 <45578715+AdamZ-8113@users.noreply.github.com> Date: Thu, 10 Sep 2026 10:42:29 -0700 Subject: [PATCH 02/14] Connect existing build comparison tests to the PoB Codes corpus (#15) Co-authored-by: Paliak <91493239+Paliak@users.noreply.github.com> --- .github/workflows/buildtest.yml | 51 +++------------------------ .github/workflows/updatebuildlist.yml | 37 ------------------- CONTRIBUTING.md | 2 +- docker-compose.yml | 25 +++++++++++++ spec/BuildDiff.sh | 14 ++++++-- spec/DiffOutput.lua | 6 ++-- spec/FetchTestBuilds.lua | 21 +++++++++++ spec/GenerateBuilds.lua | 25 +++++++------ 8 files changed, 78 insertions(+), 103 deletions(-) delete mode 100644 .github/workflows/updatebuildlist.yml create mode 100644 spec/FetchTestBuilds.lua diff --git a/.github/workflows/buildtest.yml b/.github/workflows/buildtest.yml index 6ee6ad43231..fc2b31be474 100644 --- a/.github/workflows/buildtest.yml +++ b/.github/workflows/buildtest.yml @@ -18,41 +18,17 @@ jobs: id: get-dev-ref run: | git fetch --depth=1 origin dev - echo "devref=$(git rev-parse origin/dev)" >> $GITHUB_OUTPUT + echo "devref=$(git rev-parse origin/dev)" >> "$GITHUB_OUTPUT" - name: Download Dev branch cache id: download-dev-ref-cache uses: dawidd6/action-download-artifact@3ecf4024886f219d9290351234889bfb45d1b9da with: - name: cache-devref-${{ steps.get-dev-ref.outputs.devref }} - path: /tmp/cache/ - if_no_artifact_found: warn - search_artifacts: true - # Dev ref cache contains the build list and build xmls. Use that one to keep tests reproducible - - name: Update static builds list from cache - if: ${{ steps.download-dev-ref-cache.outputs.found_artifact == 'true' }} - run: cat /tmp/cache/builds.txt > spec/builds.txt - - name: Download latest build list - if: ${{ steps.download-dev-ref-cache.outputs.found_artifact == 'false' }} - id: download-build-list - uses: dawidd6/action-download-artifact@3ecf4024886f219d9290351234889bfb45d1b9da - with: - name: builds.txt - path: /tmp/latestbuildlist/ - workflow: updatebuildlist.yml - if_no_artifact_found: warn - search_artifacts: true - - name: Update static builds list - if: ${{ steps.download-dev-ref-cache.outputs.found_artifact == 'false' && steps.download-build-list.outputs.found_artifact == 'true' }} - run: cat /tmp/latestbuildlist/builds.txt > spec/builds.txt - - name: Download latest build xmls - if: ${{ steps.download-dev-ref-cache.outputs.found_artifact == 'false' }} - uses: dawidd6/action-download-artifact@3ecf4024886f219d9290351234889bfb45d1b9da - with: - name: build-xmls + name: corpus-cache-devref-${{ steps.get-dev-ref.outputs.devref }} path: /tmp/cache/ if_no_artifact_found: warn search_artifacts: true - name: Calculate build xmls and differences between them + shell: bash run: | mkdir /tmp/cache || true # Make sure /tmp/cache exists. Ignore exit code chmod -R 777 /tmp/cache && docker compose run -v '/tmp/cache/:/cache' -e 'CACHEDIR=/cache' busted-diff | tee /tmp/dockerlog @@ -66,28 +42,9 @@ jobs: with: name: build-diff-output path: /tmp/artefact - - name: Save used build list into cache - if: ${{ steps.download-dev-ref-cache.outputs.found_artifact == 'false' }} - run: cp spec/builds.txt /tmp/cache/ - - name: Move xmls found in builds.txt to a new directory - if: ${{ steps.download-dev-ref-cache.outputs.found_artifact == 'false' && steps.download-build-list.outputs.found_artifact == 'true' }} - run: | - mkdir new-build-xmls - while IFS= read -r line; do - FILENAME="/tmp/cache/${line//[^a-zA-Z0-9]/}.xml" - if [ -f "$FILENAME" ]; then - mv "$FILENAME" "./new-build-xmls/" - fi - done < "spec/builds.txt" - - name: Upload new build xmls - if: ${{ steps.download-dev-ref-cache.outputs.found_artifact == 'false' && steps.download-build-list.outputs.found_artifact == 'true' }} - uses: actions/upload-artifact@v4 - with: - name: build-xmls - path: './new-build-xmls/*' - name: Upload dev ref cache if: ${{ steps.download-dev-ref-cache.outputs.found_artifact == 'false' }} uses: actions/upload-artifact@v4 with: - name: cache-devref-${{ steps.get-dev-ref.outputs.devref }} + name: corpus-cache-devref-${{ steps.get-dev-ref.outputs.devref }} path: /tmp/cache/ diff --git a/.github/workflows/updatebuildlist.yml b/.github/workflows/updatebuildlist.yml deleted file mode 100644 index ea841152b01..00000000000 --- a/.github/workflows/updatebuildlist.yml +++ /dev/null @@ -1,37 +0,0 @@ ---- -name: Update build list for tests -on: - schedule: - - cron: '20 4 * * *' - workflow_dispatch: -jobs: - update-builds-list: - runs-on: ubuntu-22.04 - steps: - - name: Checkout HEAD - uses: actions/checkout@v4 - - name: Install moreutils - run: sudo apt-get install -y moreutils - - name: Download latest build list - uses: dawidd6/action-download-artifact@3ecf4024886f219d9290351234889bfb45d1b9da - with: - name: builds.txt - if_no_artifact_found: warn - path: /tmp/latestbuildlist/ - - name: Update list - run: > - cat spec/builds.txt /tmp/latestbuildlist/builds.txt - <({ curl "https://pobarchives.com/api/builds?q=latest" & curl "https://pobarchives.com/api/builds?q=trending"; } - | jq -r '.builds[].build_info.build_link') - | tail -n 500 - | sort -u - | sponge builds.txt - - name: Print new builds list - run: cat builds.txt - - name: Save new build list - uses: actions/upload-artifact@v4 - with: - name: builds.txt - path: builds.txt - overwrite: true - retention-days: 3 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ba990338634..b884cfbe53c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -305,7 +305,7 @@ While both can be ran locally it's recommended to use the provided docker image PoB uses the [Busted](https://lunarmodules.github.io/busted/) framework to run its tests. Tests are stored under `spec/System` and run automatically when a PR is modified. More tests can be added to this folder to test specific functionality, or new test builds can be added to ensure nothing changed that wasn't intended. -### Running tests +To run the build difference tests run `docker compose up busted-diff`. `busted-diff` first downloads the build corpus from `https://api.pob.codes/test-builds/corpus` and computes it alongside `spec/TestBuilds/` using the current working changes. Then it uses git to checkout the branch specified by `DEVREF` or `dev` by default and computes the same builds. Lastly it finds the differences in the outputs generated from both, does some post processing, and prints all of the information out. The downloaded builds and list stay with the calculated `DEVREF` cache and are reused together until that cache changes. 1. Install [Docker](https://www.docker.com/get-started) 2. Run `docker-compose up` from the command line diff --git a/docker-compose.yml b/docker-compose.yml index b01cfb360ab..273407c83e8 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -13,3 +13,28 @@ services: working_dir: /workdir volumes: - ./:/workdir:ro + busted-diff: + #build: . + image: ghcr.io/pathofbuildingcommunity/pathofbuilding-tests:latest + environment: #Where in the container the folders are stored + WORKDIR: /workdir + HOME: /tmp + DEVREF: ${DEVREF:-origin/dev} + HEADREF: $HEADREF + LUA_PATH: "/usr/share/lua/5.1/?.lua;/usr/share/lua/5.1/?/init.lua;;" + LUA_CPATH: "/usr/lib/lua/5.1/?.so;;" + container_name: busted-diff + tty: true + # Dependencies used by the existing build-diff scripts; the image provides LuaJIT. + command: + - /bin/sh + - -ec + - | + apk add --no-cache git parallel libxml2-utils lua5.1-posix lua5.1-curl zlib-dev + luarocks install lua-zlib 1.4-0 + exec su -p nobody -s /bin/sh -c 'dos2unix < /workdir/spec/BuildDiff.sh | /bin/sh' + security_opt: + - no-new-privileges:true + working_dir: /workdir + volumes: + - ./:/workdir:ro diff --git a/spec/BuildDiff.sh b/spec/BuildDiff.sh index 180960a11d1..05768e07eb4 100644 --- a/spec/BuildDiff.sh +++ b/spec/BuildDiff.sh @@ -1,4 +1,5 @@ #!/bin/sh +set -eo pipefail umask 0 # If external cache dir has not been defined keep it inside the container @@ -17,14 +18,21 @@ git config --global --add advice.detachedHead false if [[ ! -z "$HEADREF" ]] then git diff --no-color "$HEADREF" -- /tmp/workdir/.busted /tmp/workdir/src/HeadlessWrapper.lua /tmp/workdir/spec/ > /tmp/HeadPatch && - git reset --hard "$HEADREF" && git clean -fd && git apply --allow-empty /tmp/HeadPatch + git reset --hard "$HEADREF" && git clean -fd && git apply --allow-empty --index /tmp/HeadPatch fi headsha=$(git rev-parse HEAD) devsha=$(git rev-parse "$DEVREF") +# Keep the input corpus with the calculated base, as in the existing workflow. +if [[ ! -f "$CACHEDIR/$devsha" ]]; then + curl --fail --show-error --silent https://api.pob.codes/test-builds/corpus -o "$CACHEDIR/corpus.json" + luajit spec/FetchTestBuilds.lua "$CACHEDIR" +fi +cp "$CACHEDIR/builds.txt" spec/builds.txt + rm -rf /tmp/headsha && mkdir /tmp/headsha -rm /tmp/workdir/src/Settings.xml +rm -f /tmp/workdir/src/Settings.xml cat /tmp/workdir/spec/builds.txt | dos2unix | parallel --will-cite --ungroup --pipe -N50 'LINKSBATCH="$(mktemp){#}"; cat > $LINKSBATCH; BUILDLINKS="$LINKSBATCH" BUILDCACHEPREFIX="/tmp/headsha" busted --lua=luajit -r generate' && \ BUILDCACHEPREFIX='/tmp/headsha' busted --lua=luajit -r generate && date > "/tmp/headsha/$headsha" && echo "[+] Build cache computed for $headsha (headsha)" || exit $? @@ -34,7 +42,7 @@ then # Keep new changes to tests related files git diff --no-color "$DEVREF" -- /tmp/workdir/.busted /tmp/workdir/src/HeadlessWrapper.lua /tmp/workdir/spec/ > /tmp/DevPatch && \ - git reset --hard "$DEVREF" && git clean -fd && git apply --allow-empty /tmp/DevPatch && \ + git reset --hard "$DEVREF" && git clean -fd && git apply --allow-empty --index /tmp/DevPatch && \ cat /tmp/workdir/spec/builds.txt | dos2unix | parallel --will-cite --ungroup --pipe -N50 'LINKSBATCH="$(mktemp){#}"; cat > $LINKSBATCH; BUILDLINKS="$LINKSBATCH" BUILDCACHEPREFIX="$CACHEDIR" busted --lua=luajit -r generate' && \ BUILDCACHEPREFIX="$CACHEDIR" busted --lua=luajit -r generate && date > "$CACHEDIR/$devsha" && echo "[+] Build cache computed for $devsha (devsha)" || exit $? fi diff --git a/spec/DiffOutput.lua b/spec/DiffOutput.lua index 5f3228f21c0..949c36ba44e 100644 --- a/spec/DiffOutput.lua +++ b/spec/DiffOutput.lua @@ -8,11 +8,13 @@ local function buildOutputMap(filecontent) local playerOutput = {} local minionOutput = {} for line in splitLines(filecontent) do - local key, val = line:match('PlayerStat stat="(.-)" value="(.-)"') + local stat = line:match(']+)') + local key, val = stat and stat:match('stat="(.-)"'), stat and stat:match('value="(.-)"') if key then playerOutput[key] = val else - local key,val = line:match('MinionStat stat="(.-)" value="(.-)"') + local stat = line:match(']+)') + local key, val = stat and stat:match('stat="(.-)"'), stat and stat:match('value="(.-)"') if key then minionOutput[key] = val end diff --git a/spec/FetchTestBuilds.lua b/spec/FetchTestBuilds.lua new file mode 100644 index 00000000000..82123f141ba --- /dev/null +++ b/spec/FetchTestBuilds.lua @@ -0,0 +1,21 @@ +-- Store the API corpus alongside the calculated base cache, outside Git. +package.path = "runtime/lua/?.lua;" .. package.path +local json = require("dkjson") +local base64 = require("base64") +local zlib = require("zlib") +local cache = assert(arg[1]) +local input = assert(io.open(cache .. "/corpus.json", "r")) +local corpus = assert(json.decode(input:read("*a"))) +input:close() +assert(corpus.schemaVersion == 2 and #corpus.builds > 0 and #corpus.builds == corpus.count, "Invalid build corpus") +local list = assert(io.open(cache .. "/builds.txt", "w")) +for _, entry in ipairs(corpus.builds) do + assert(#entry.sha256 == 64 and entry.sha256:match("^%x+$"), "Invalid build filename") + local xml = zlib.inflate()(base64.decode(entry.code:gsub("-", "+"):gsub("_", "/"))) + local output = assert(io.open(cache .. "/" .. entry.sha256 .. ".xml", "w")) + output:write(xml) + output:close() + list:write(entry.sha256, "\n") +end +list:close() +print("[+] Downloaded " .. corpus.count .. " builds from corpus " .. corpus.snapshotId) diff --git a/spec/GenerateBuilds.lua b/spec/GenerateBuilds.lua index 9ed6c9de924..dc3be4fe325 100644 --- a/spec/GenerateBuilds.lua +++ b/spec/GenerateBuilds.lua @@ -8,16 +8,15 @@ local function fetchBuilds(path) fileHnd:close() for line in splitLines(fileText) do if line ~= "" then - for j = 1, #buildSites.websiteList do - if line:match(buildSites.websiteList[j].matchURL) then - local filename = line:gsub('%W', '') - - -- Load from cache if downloaded already - local fileHnd = io.open( (os.getenv("CACHEDIR") or "/tmp") .. "/" .. filename .. ".xml", "r") - if fileHnd then - coroutine.yield({ xml = fileHnd:read("*a"), filename = filename, link = line }) - fileHnd:close() - else + local filename = line:gsub('%W', '') + -- Check cached XML before matching a download provider. + local fileHnd = io.open((os.getenv("CACHEDIR") or "/tmp") .. "/" .. filename .. ".xml", "r") + if fileHnd then + coroutine.yield({ xml = fileHnd:read("*a"), filename = filename, link = line }) + fileHnd:close() + else + for j = 1, #buildSites.websiteList do + if line:match(buildSites.websiteList[j].matchURL) then -- Throttle build downloads to 15 per 10 seconds local timeSinceLastDL = GetTime() - lastDLtime if timeSinceLastDL < 666 then @@ -35,10 +34,10 @@ local function fetchBuilds(path) print("Failed to download build: " .. line) end end) + break + elseif j == #buildSites.websiteList then + print("Failed to match provider for: " .. line) end - break - elseif j == #buildSites.websiteList then - print("Failed to match provider for: " .. line) end end end From 40dd96dcaf8192b747988667464d56d2664aa7a3 Mon Sep 17 00:00:00 2001 From: Paliak <91493239+Paliak@users.noreply.github.com> Date: Thu, 10 Sep 2026 19:50:44 +0200 Subject: [PATCH 03/14] Rename builddiff.yaml action --- .github/workflows/buildtest.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/buildtest.yml b/.github/workflows/buildtest.yml index fc2b31be474..e6c9c115da4 100644 --- a/.github/workflows/buildtest.yml +++ b/.github/workflows/buildtest.yml @@ -1,5 +1,5 @@ --- -name: Run Tests +name: Run Build Diff Tests on: pull_request: branches: From 9201d9b9cb67bf9d16db48d858f1666fd4ade833 Mon Sep 17 00:00:00 2001 From: Paliak <91493239+Paliak@users.noreply.github.com> Date: Thu, 10 Sep 2026 19:53:03 +0200 Subject: [PATCH 04/14] Remove merge conflict leftovers --- src/HeadlessWrapper.lua | 1 - 1 file changed, 1 deletion(-) diff --git a/src/HeadlessWrapper.lua b/src/HeadlessWrapper.lua index 5f246c4a8e3..4c09db95cfc 100644 --- a/src/HeadlessWrapper.lua +++ b/src/HeadlessWrapper.lua @@ -59,7 +59,6 @@ function GetVirtualScreenSize() return 1920, 1080 end -<<<<<<< HEAD posix = require("posix") -- Search Handles From 59c25d2381f3965c6cd2b019c9d06534fc4a15b3 Mon Sep 17 00:00:00 2001 From: Paliak <91493239+Paliak@users.noreply.github.com> Date: Thu, 10 Sep 2026 20:14:22 +0200 Subject: [PATCH 05/14] Script CI header edit due to missing __mainObject__ --- src/HeadlessWrapper.lua | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/HeadlessWrapper.lua b/src/HeadlessWrapper.lua index 4c09db95cfc..5d2d0841322 100644 --- a/src/HeadlessWrapper.lua +++ b/src/HeadlessWrapper.lua @@ -146,9 +146,6 @@ end dofile("Launch.lua") --- The CI env var will be true when run from github workflows but should be false for other tools using the headless wrapper -__mainObject__.continuousIntegrationMode = os.getenv("CI") - function launch:DownloadPage(url, callback, params) params = params or {} local responseHeader = "" @@ -164,7 +161,7 @@ function launch:DownloadPage(url, callback, params) easy:setopt(curl.OPT_HTTPHEADER, header) end easy:setopt_url(url) - easy:setopt(curl.OPT_USERAGENT, "Headless Path of Building" .. (mainObject.continuousIntegrationMode and " CI" or "") .. "/"..launch.versionNumber) + easy:setopt(curl.OPT_USERAGENT, "Headless Path of Building" .. "/"..launch.versionNumber) easy:setopt(curl.OPT_ACCEPT_ENCODING, "") if params.body then easy:setopt(curl.OPT_POST, true) From 7bbf2c7c35811aed3204549e010e374d5f03e28a Mon Sep 17 00:00:00 2001 From: AdamZ Date: Thu, 10 Sep 2026 20:35:44 -0700 Subject: [PATCH 06/14] Fix headless startup by removing old callback definitions --- src/HeadlessWrapper.lua | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/src/HeadlessWrapper.lua b/src/HeadlessWrapper.lua index 5d2d0841322..f35f04c3336 100644 --- a/src/HeadlessWrapper.lua +++ b/src/HeadlessWrapper.lua @@ -7,26 +7,6 @@ -- bodies intended for headless use. dofile("_SimpleGraphic.def.lua") --- Callbacks -local callbackTable = { } -local mainObject -function runCallback(name, ...) - if callbackTable[name] then - return callbackTable[name](...) - elseif mainObject and mainObject[name] then - return mainObject[name](mainObject, ...) - end -end -function SetCallback(name, func) - callbackTable[name] = func -end -function GetCallback(name) - return callbackTable[name] -end -function SetMainObject(obj) - mainObject = obj -end - -- https://stackoverflow.com/questions/19326368/iterate-over-lines-including-blank-lines function splitLines(s) if s:sub(-1)~="\n" then s=s.."\n" end From 39e1071f7e64058df46505fa7701216567452cf9 Mon Sep 17 00:00:00 2001 From: AdamZ Date: Thu, 10 Sep 2026 20:42:18 -0700 Subject: [PATCH 07/14] Export full calculated stats consistently in build tests --- spec/BuildStats.lua | 33 +++++++++++++++++++++++++++++++++ spec/GenerateBuilds.lua | 14 +++++++++++++- spec/System/BuildStats_spec.lua | 29 +++++++++++++++++++++++++++++ 3 files changed, 75 insertions(+), 1 deletion(-) create mode 100644 spec/BuildStats.lua create mode 100644 spec/System/BuildStats_spec.lua diff --git a/spec/BuildStats.lua b/spec/BuildStats.lua new file mode 100644 index 00000000000..dbf8e70bbe0 --- /dev/null +++ b/spec/BuildStats.lua @@ -0,0 +1,33 @@ +-- Append calculated stats to the normal saved XML. Item/gem source references +-- describe objects, not calculated stats, and can lead back into the object graph. +return function(xml, output, element) + local stats, active = {}, {} + local function collect(values, prefix) + if active[values] or rawget(values, "Object") == values then return end + active[values] = true + for key, value in pairs(values) do + if type(key) == "string" or type(key) == "number" then + local name = prefix .. key + if type(value) == "table" then + if key ~= "sourceItem" and key ~= "sourceGem" and not (prefix == "" and key == "Minion") then + collect(value, name .. ".") + end + elseif type(value) == "number" or type(value) == "string" or type(value) == "boolean" then + stats[name] = tostring(value) + end + end + end + active[values] = nil + end + collect(output, "") + -- Preserve stats already emitted by PoB's normal save path. + for _, child in ipairs(xml) do + if child.elem == element then stats[child.attrib.stat] = nil end + end + local names = {} + for name in pairs(stats) do names[#names + 1] = name end + table.sort(names) + for _, name in ipairs(names) do + xml[#xml + 1] = { elem = element, attrib = { stat = name, value = stats[name] } } + end +end diff --git a/spec/GenerateBuilds.lua b/spec/GenerateBuilds.lua index dc3be4fe325..d4cf5883929 100644 --- a/spec/GenerateBuilds.lua +++ b/spec/GenerateBuilds.lua @@ -1,3 +1,15 @@ +-- Use the same test exporter on both revisions, including bases whose normal +-- SaveDB does not support the old fullPlayerStat/fullMinionStat options. +local appendStats = dofile("../spec/BuildStats.lua") +local saveBuild = build.Save +function build:Save(xml) + saveBuild(self, xml) + appendStats(xml, self.calcsTab.mainOutput, "PlayerStat") + if self.calcsTab.mainOutput.Minion then + appendStats(xml, self.calcsTab.mainOutput.Minion, "MinionStat") + end +end + local function fetchBuilds(path) local lastDLtime = GetTime() local co = coroutine.create(function(path) @@ -78,7 +90,7 @@ for testBuild in fetchBuilds("../spec/TestBuilds") do -- Save the computed build xml. Include full minion and player outputs. local buildHnd = io.open(filePath .. ".build", "w+") - buildHnd:write(build:SaveDB("Cache", {fullPlayerStat = true, fullMinionStat = true} )) + buildHnd:write(build:SaveDB("Cache")) buildHnd:close() -- Save the amount of time calculation of this build took diff --git a/spec/System/BuildStats_spec.lua b/spec/System/BuildStats_spec.lua new file mode 100644 index 00000000000..2ad6464a7a1 --- /dev/null +++ b/spec/System/BuildStats_spec.lua @@ -0,0 +1,29 @@ +describe("Full build stat export", function() + local appendStats = dofile("../spec/BuildStats.lua") + it("keeps both nested stat paths and minion stats", function() + local xml = { { elem = "PlayerStat", attrib = { stat = "Life", value = "100" } } } + local output = { Life = 100, MainHand = { Damage = 10 }, OffHand = { Damage = 20 }, Minion = { Life = 50 } } + appendStats(xml, output, "PlayerStat") + appendStats(xml, output.Minion, "MinionStat") + assert.same({ + { elem = "PlayerStat", attrib = { stat = "Life", value = "100" } }, + { elem = "PlayerStat", attrib = { stat = "MainHand.Damage", value = "10" } }, + { elem = "PlayerStat", attrib = { stat = "OffHand.Damage", value = "20" } }, + { elem = "MinionStat", attrib = { stat = "Life", value = "50" } }, + }, xml) + end) + it("avoids runtime object references and cycles without dropping shared stats", function() + local object = { internal = 99 } + object.Object = object + local shared = { Damage = 10 } + local output = { A = shared, B = shared, ObjectReference = object, Requirement = { value = 42, sourceItem = object, sourceGem = { internal = 99 } }, [object] = true } + output.Cycle = output + local xml = {} + appendStats(xml, output, "PlayerStat") + assert.same({ + { elem = "PlayerStat", attrib = { stat = "A.Damage", value = "10" } }, + { elem = "PlayerStat", attrib = { stat = "B.Damage", value = "10" } }, + { elem = "PlayerStat", attrib = { stat = "Requirement.value", value = "42" } }, + }, xml) + end) +end) From 6fd95af148de9866d5a51b610574d8b596637af8 Mon Sep 17 00:00:00 2001 From: AdamZ Date: Thu, 10 Sep 2026 20:42:18 -0700 Subject: [PATCH 08/14] Mock the actual download method in trade request tests --- spec/System/TestTradeQueryRequests_spec.lua | 22 ++++++++------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/spec/System/TestTradeQueryRequests_spec.lua b/spec/System/TestTradeQueryRequests_spec.lua index f05068d8fa2..f57ce72d151 100644 --- a/spec/System/TestTradeQueryRequests_spec.lua +++ b/spec/System/TestTradeQueryRequests_spec.lua @@ -46,12 +46,10 @@ describe("TradeQueryRequests", function() -- Pass: Dequeues and processes valid item -- Fail: Queue unchanged, indicating timing/insertion bug, blocking trade searches it("processes search queue item", function() - local orig_launch = launch - launch = { - DownloadPage = function(url, onComplete, opts) - onComplete({ body = "{}", header = "HTTP/1.1 200 OK" }, nil) - end - } + local download = stub(launch, "DownloadPage", function(self, url, onComplete, opts) + onComplete({ body = "{}", header = "HTTP/1.1 200 OK" }, nil) + end) + finally(function() download:revert() end) table.insert(requests.requestQueue.search, { url = "test", callback = function() end, @@ -63,7 +61,6 @@ describe("TradeQueryRequests", function() mock_limiter.NextRequestTime = mock_next_time requests:ProcessQueue() assert.are.equal(#requests.requestQueue.search, 0) - launch = orig_launch end) -- Pass: Does not crash on 401, and passes error message @@ -79,12 +76,10 @@ Server: cloudflare WWW-Authenticate: Bearer realm="pathofexile:production", error="invalid_token", error_description="The access token provided is invalid or has expired" Cache-Control: no-store Strict-Transport-Security: max-age=63115200; includeSubDomains; preload]] - local orig_launch = launch - launch = { - DownloadPage = function(url, onComplete, opts) - onComplete({ body = json, header = header }, nil) - end - } + local download = stub(launch, "DownloadPage", function(self, url, onComplete, opts) + onComplete({ body = json, header = header }, "Response code: 401") + end) + finally(function() download:revert() end) table.insert(requests.requestQueue.search, { url = "test", callback = function(body, msg) @@ -99,7 +94,6 @@ Strict-Transport-Security: max-age=63115200; includeSubDomains; preload]] mock_limiter.NextRequestTime = mock_next_time requests:ProcessQueue() assert.are.equal(#requests.requestQueue.search, 0) - launch = orig_launch end) -- Pass: Retries with increasing backoff up to cap, preventing infinite loops From 829297556a16d9ec689ad27b833a5a5595e4d4a3 Mon Sep 17 00:00:00 2001 From: AdamZ Date: Thu, 10 Sep 2026 20:42:18 -0700 Subject: [PATCH 09/14] Make the test cache writable for the container user --- docker-compose.yml | 32 ++++++++++++++++++++++---------- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index 273407c83e8..00ea6023a79 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -4,6 +4,21 @@ services: image: ghcr.io/pathofbuildingcommunity/pathofbuilding-tests:latest environment: HOME: /tmp + LUA_PATH: "/usr/share/lua/5.1/?.lua;/usr/share/lua/5.1/?/init.lua;;" + LUA_CPATH: "/usr/lib/lua/5.1/?.so;;" + # The official image supplies LuaJIT; these are the existing script dependencies. + entrypoint: &test-entrypoint + - /bin/sh + - -ec + - | + apk add --no-cache git parallel libxml2-utils lua5.1-posix lua5.1-curl zlib-dev + luarocks install lua-zlib 1.4-0 + if [ -n "$${CACHEDIR:-}" ]; then + mkdir -p "$$CACHEDIR" + chown nobody:nobody "$$CACHEDIR" + fi + exec su -p nobody -s /bin/sh -c "$$*" + - -- container_name: pathofbuilding-tests command: busted --lua=luajit security_opt: @@ -19,22 +34,19 @@ services: environment: #Where in the container the folders are stored WORKDIR: /workdir HOME: /tmp + CACHEDIR: /cache DEVREF: ${DEVREF:-origin/dev} - HEADREF: $HEADREF + HEADREF: ${HEADREF:-} LUA_PATH: "/usr/share/lua/5.1/?.lua;/usr/share/lua/5.1/?/init.lua;;" LUA_CPATH: "/usr/lib/lua/5.1/?.so;;" container_name: busted-diff - tty: true - # Dependencies used by the existing build-diff scripts; the image provides LuaJIT. - command: - - /bin/sh - - -ec - - | - apk add --no-cache git parallel libxml2-utils lua5.1-posix lua5.1-curl zlib-dev - luarocks install lua-zlib 1.4-0 - exec su -p nobody -s /bin/sh -c 'dos2unix < /workdir/spec/BuildDiff.sh | /bin/sh' + entrypoint: *test-entrypoint + command: ["dos2unix < /workdir/spec/BuildDiff.sh | /bin/sh"] security_opt: - no-new-privileges:true working_dir: /workdir volumes: - ./:/workdir:ro + - build-cache:/cache +volumes: + build-cache: From ac1b243cefea4b8d30d2613289673b06a78482ec Mon Sep 17 00:00:00 2001 From: AdamZ Date: Thu, 10 Sep 2026 20:47:07 -0700 Subject: [PATCH 10/14] Reuse one baseline cache and prepare it nightly --- .gitattributes | 3 +- .github/workflows/buildtest.yml | 101 +++++++++++++++++-------- CONTRIBUTING.md | 2 +- docker-compose.yml | 4 +- spec/BuildCache.sh | 13 ++++ spec/BuildDiff.sh | 129 ++++++++++++++++---------------- spec/GenerateBuilds.lua | 121 +++++++++--------------------- 7 files changed, 191 insertions(+), 182 deletions(-) create mode 100644 spec/BuildCache.sh diff --git a/.gitattributes b/.gitattributes index 16abb938292..04ffb4ef1a7 100644 --- a/.gitattributes +++ b/.gitattributes @@ -2,6 +2,7 @@ # Set default behavior to automatically normalize line endings. ############################################################################### * text=auto +spec/*.sh text eol=lf ############################################################################### # Set default behavior for command prompt diff. @@ -66,4 +67,4 @@ # Executable files - preserve execution permissions on Unix systems # (https://git-scm.com/docs/gitattributes#_executable) ############################################################################### -runtime/*.exe binary \ No newline at end of file +runtime/*.exe binary diff --git a/.github/workflows/buildtest.yml b/.github/workflows/buildtest.yml index e6c9c115da4..9037636e1a9 100644 --- a/.github/workflows/buildtest.yml +++ b/.github/workflows/buildtest.yml @@ -1,50 +1,89 @@ ---- name: Run Build Diff Tests on: pull_request: - branches: - - dev + branches: [dev] + schedule: + - cron: '17 4 * * *' workflow_dispatch: + inputs: + mode: + description: Compare builds or prepare the shared baseline + type: choice + options: [compare, prepare] + default: compare + base_ref: + description: Base branch or commit + default: dev +permissions: + contents: read concurrency: - group: ${{ github.workflow }}-${{ github.ref }} + group: build-diff-${{ github.event_name == 'pull_request' && github.ref || 'baseline' }} cancel-in-progress: true jobs: run_build_diff: runs-on: ubuntu-latest + timeout-minutes: 20 + env: + BASE_ONLY: ${{ (github.event_name == 'schedule' || inputs.mode == 'prepare') && '1' || '0' }} steps: - - name: Checkout HEAD + - name: Checkout test code uses: actions/checkout@v4 - - name: Fetch Dev branch - id: get-dev-ref + with: + persist-credentials: false + - name: Select base and download the corpus once + id: inputs + env: + BASE_REF: ${{ github.event.pull_request.base.sha || inputs.base_ref || 'dev' }} + shell: bash run: | - git fetch --depth=1 origin dev - echo "devref=$(git rev-parse origin/dev)" >> "$GITHUB_OUTPUT" - - name: Download Dev branch cache - id: download-dev-ref-cache - uses: dawidd6/action-download-artifact@3ecf4024886f219d9290351234889bfb45d1b9da + git fetch --depth=1 origin "$BASE_REF" + export DEVREF=$(git rev-parse FETCH_HEAD) + source spec/BuildCache.sh + echo "base=$DEV_SHA" >> "$GITHUB_OUTPUT" + echo "key=$CACHE_KEY" >> "$GITHUB_OUTPUT" + echo "corpus=$CORPUS_FILE" >> "$GITHUB_OUTPUT" + echo "Baseline cache: $CACHE_KEY" + - name: Find or restore the calculated base + id: cache + uses: actions/cache/restore@v4 + continue-on-error: true with: - name: corpus-cache-devref-${{ steps.get-dev-ref.outputs.devref }} - path: /tmp/cache/ - if_no_artifact_found: warn - search_artifacts: true - - name: Calculate build xmls and differences between them + key: ${{ steps.inputs.outputs.key }} + path: /tmp/pob-cache/${{ steps.inputs.outputs.key }} + lookup-only: ${{ env.BASE_ONLY == '1' }} + - name: Calculate builds + id: calculate + if: env.BASE_ONLY != '1' || steps.cache.outputs.cache-hit != 'true' + env: + DEVREF: ${{ steps.inputs.outputs.base }} + CORPUS_FILE: ${{ steps.inputs.outputs.corpus }} + CACHE_KEY: ${{ steps.inputs.outputs.key }} shell: bash run: | - mkdir /tmp/cache || true # Make sure /tmp/cache exists. Ignore exit code - chmod -R 777 /tmp/cache && docker compose run -v '/tmp/cache/:/cache' -e 'CACHEDIR=/cache' busted-diff | tee /tmp/dockerlog - - name: Generate artefact + mkdir -p /tmp/pob-cache + docker compose run --rm -v /tmp/pob-cache:/cache \ + -v "$CORPUS_FILE:/corpus.json:ro" -e CORPUS_FILE=/corpus.json \ + busted-diff | tee /tmp/dockerlog + test -f "/tmp/pob-cache/$CACHE_KEY/$DEVREF" + - name: Save the completed base + if: success() && steps.cache.outputs.cache-hit != 'true' + uses: actions/cache/save@v4 + continue-on-error: true + with: + key: ${{ steps.inputs.outputs.key }} + path: /tmp/pob-cache/${{ steps.inputs.outputs.key }} + - name: Collect comparison output + if: always() && env.BASE_ONLY != '1' && steps.calculate.outcome != 'skipped' + shell: bash run: | - sed -n '/Runtime comparison for/,/Savefile Diff for/{/Savefile Diff for/!p;}' /tmp/dockerlog > /tmp/artefact - sed -n '/Savefile Diff for/, $p' /tmp/dockerlog >> /tmp/artefact - [ -s /tmp/artefact ] || rm /tmp/artefact - - name: Upload artefact + if [ -f /tmp/dockerlog ]; then + sed -n '/^## /,$p' /tmp/dockerlog > /tmp/build-diff-output + [ -s /tmp/build-diff-output ] || rm -f /tmp/build-diff-output + fi + - name: Upload comparison output + if: always() && env.BASE_ONLY != '1' uses: actions/upload-artifact@v4 with: name: build-diff-output - path: /tmp/artefact - - name: Upload dev ref cache - if: ${{ steps.download-dev-ref-cache.outputs.found_artifact == 'false' }} - uses: actions/upload-artifact@v4 - with: - name: corpus-cache-devref-${{ steps.get-dev-ref.outputs.devref }} - path: /tmp/cache/ + path: /tmp/build-diff-output + if-no-files-found: ignore diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b884cfbe53c..87710a04ee2 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -305,7 +305,7 @@ While both can be ran locally it's recommended to use the provided docker image PoB uses the [Busted](https://lunarmodules.github.io/busted/) framework to run its tests. Tests are stored under `spec/System` and run automatically when a PR is modified. More tests can be added to this folder to test specific functionality, or new test builds can be added to ensure nothing changed that wasn't intended. -To run the build difference tests run `docker compose up busted-diff`. `busted-diff` first downloads the build corpus from `https://api.pob.codes/test-builds/corpus` and computes it alongside `spec/TestBuilds/` using the current working changes. Then it uses git to checkout the branch specified by `DEVREF` or `dev` by default and computes the same builds. Lastly it finds the differences in the outputs generated from both, does some post processing, and prints all of the information out. The downloaded builds and list stay with the calculated `DEVREF` cache and are reused together until that cache changes. +To run the build difference tests, first fetch `origin/dev`, then run `docker compose run --rm busted-diff`. Set `DEVREF` to compare against another branch or commit. The test uses the same PoB Codes builds and checked-in fixtures on both revisions, and reports stat, saved-XML and timing differences. Base results are reused from the Docker `build-cache` volume when the base, corpus and test tools match; `docker compose down --volumes` clears that cache. GitHub Actions prepares the shared baseline nightly, while PRs calculate both sides themselves if it is unavailable. 1. Install [Docker](https://www.docker.com/get-started) 2. Run `docker-compose up` from the command line diff --git a/docker-compose.yml b/docker-compose.yml index 00ea6023a79..bff1d081a3d 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -15,7 +15,7 @@ services: luarocks install lua-zlib 1.4-0 if [ -n "$${CACHEDIR:-}" ]; then mkdir -p "$$CACHEDIR" - chown nobody:nobody "$$CACHEDIR" + chown -R nobody:nobody "$$CACHEDIR" fi exec su -p nobody -s /bin/sh -c "$$*" - -- @@ -35,6 +35,8 @@ services: WORKDIR: /workdir HOME: /tmp CACHEDIR: /cache + BASE_ONLY: ${BASE_ONLY:-0} + BUILD_JOBS: ${BUILD_JOBS:-2} DEVREF: ${DEVREF:-origin/dev} HEADREF: ${HEADREF:-} LUA_PATH: "/usr/share/lua/5.1/?.lua;/usr/share/lua/5.1/?/init.lua;;" diff --git a/spec/BuildCache.sh b/spec/BuildCache.sh new file mode 100644 index 00000000000..df19e039c0a --- /dev/null +++ b/spec/BuildCache.sh @@ -0,0 +1,13 @@ +# Shared by GitHub Actions and the local runner so cache identity is identical. +DEV_SHA=$(git rev-parse "${DEVREF:-origin/dev}^{commit}") +if [ -z "${CORPUS_FILE:-}" ]; then + CORPUS_FILE=$(mktemp) + curl --fail --show-error --silent --max-time 60 \ + https://api.pob.codes/test-builds/corpus -o "$CORPUS_FILE" +fi +corpus_hash=$(sha256sum "$CORPUS_FILE" | cut -d ' ' -f 1) +test_hash=$(git ls-files -z -- .busted docker-compose.yml src/HeadlessWrapper.lua \ + spec/BuildCache.sh spec/BuildDiff.sh spec/BuildStats.lua spec/FetchTestBuilds.lua \ + spec/GenerateBuilds.lua spec/TestBuilds | xargs -0 git hash-object -- | sha256sum | cut -d ' ' -f 1) +CACHE_KEY="pob-builds-v1-$DEV_SHA-$corpus_hash-$test_hash" +export DEV_SHA CORPUS_FILE CACHE_KEY diff --git a/spec/BuildDiff.sh b/spec/BuildDiff.sh index 05768e07eb4..bd9fe62c6d2 100644 --- a/spec/BuildDiff.sh +++ b/spec/BuildDiff.sh @@ -2,80 +2,81 @@ set -eo pipefail umask 0 -# If external cache dir has not been defined keep it inside the container -if [[ -z "$CACHEDIR" ]] -then - mkdir /tmp/cachedir - export CACHEDIR="/tmp/cachedir" -fi - -# Copy mounted workdir to allow for changes during test run -rm -rf /tmp/workdir && mkdir /tmp/workdir && cp -rf "$WORKDIR"/. /tmp/workdir/ && cd /tmp/workdir - +# Work on a copy: checking out the base must not change the user's checkout. +rm -rf /tmp/workdir +mkdir /tmp/workdir +cp -rf "$WORKDIR"/. /tmp/workdir/ +cd /tmp/workdir git config --global --add safe.directory /tmp/workdir git config --global --add advice.detachedHead false - -if [[ ! -z "$HEADREF" ]] -then - git diff --no-color "$HEADREF" -- /tmp/workdir/.busted /tmp/workdir/src/HeadlessWrapper.lua /tmp/workdir/spec/ > /tmp/HeadPatch && - git reset --hard "$HEADREF" && git clean -fd && git apply --allow-empty --index /tmp/HeadPatch +if [ -n "$HEADREF" ]; then + git diff --binary "$HEADREF" -- .busted src/HeadlessWrapper.lua spec/ > /tmp/HeadPatch + git reset --hard "$HEADREF" + git clean -fd + git apply --allow-empty --index /tmp/HeadPatch fi - headsha=$(git rev-parse HEAD) -devsha=$(git rev-parse "$DEVREF") -# Keep the input corpus with the calculated base, as in the existing workflow. -if [[ ! -f "$CACHEDIR/$devsha" ]]; then - curl --fail --show-error --silent https://api.pob.codes/test-builds/corpus -o "$CACHEDIR/corpus.json" +# The same response and test tools identify the inputs and calculated base. +. ./spec/BuildCache.sh +export CACHEDIR="${CACHEDIR:-/tmp/cachedir}/$CACHE_KEY" +mkdir -p "$CACHEDIR" +echo "[+] Baseline cache: $CACHE_KEY" +if [ ! -f "$CACHEDIR/$DEV_SHA" ]; then + rm -f "$CACHEDIR"/*.build "$CACHEDIR"/*.time + cp "$CORPUS_FILE" "$CACHEDIR/corpus.json" luajit spec/FetchTestBuilds.lua "$CACHEDIR" fi cp "$CACHEDIR/builds.txt" spec/builds.txt -rm -rf /tmp/headsha && mkdir /tmp/headsha -rm -f /tmp/workdir/src/Settings.xml -cat /tmp/workdir/spec/builds.txt | dos2unix | parallel --will-cite --ungroup --pipe -N50 'LINKSBATCH="$(mktemp){#}"; cat > $LINKSBATCH; BUILDLINKS="$LINKSBATCH" BUILDCACHEPREFIX="/tmp/headsha" busted --lua=luajit -r generate' && \ -BUILDCACHEPREFIX='/tmp/headsha' busted --lua=luajit -r generate && date > "/tmp/headsha/$headsha" && echo "[+] Build cache computed for $headsha (headsha)" || exit $? - -if [[ ! -f "$CACHEDIR/$devsha" ]] # Output of builds outdated or nonexistent -then - rm -rf "$CACHEDIR"/*.build +# Restart PoB after each batch of 50, then calculate the checked-in fixtures. +calculate() { + export BUILDCACHEPREFIX="$1" + mkdir -p "$BUILDCACHEPREFIX" + cat spec/builds.txt | dos2unix | parallel --jobs "${BUILD_JOBS:-2}" --halt soon,fail=1 --will-cite --ungroup --pipe -N50 \ + 'batch=$(mktemp); cat > "$batch"; BUILDLINKS="$batch" busted --lua=luajit -r generate' + busted --lua=luajit -r generate + expected=$(( $(wc -l < spec/builds.txt) + $(find spec/TestBuilds -maxdepth 1 -name '*.xml' | wc -l) )) + actual=$(find "$BUILDCACHEPREFIX" -maxdepth 1 -name '*.build' | wc -l) + [ "$actual" -eq "$expected" ] || { echo "Expected $expected saved builds; got $actual" >&2; exit 1; } + echo "[+] Calculated $actual builds into $BUILDCACHEPREFIX" +} - # Keep new changes to tests related files - git diff --no-color "$DEVREF" -- /tmp/workdir/.busted /tmp/workdir/src/HeadlessWrapper.lua /tmp/workdir/spec/ > /tmp/DevPatch && \ - git reset --hard "$DEVREF" && git clean -fd && git apply --allow-empty --index /tmp/DevPatch && \ - cat /tmp/workdir/spec/builds.txt | dos2unix | parallel --will-cite --ungroup --pipe -N50 'LINKSBATCH="$(mktemp){#}"; cat > $LINKSBATCH; BUILDLINKS="$LINKSBATCH" BUILDCACHEPREFIX="$CACHEDIR" busted --lua=luajit -r generate' && \ - BUILDCACHEPREFIX="$CACHEDIR" busted --lua=luajit -r generate && date > "$CACHEDIR/$devsha" && echo "[+] Build cache computed for $devsha (devsha)" || exit $? +# Normal PR/local runs calculate head. The nightly job only prepares the base. +if [ "${BASE_ONLY:-0}" != 1 ]; then + rm -rf /tmp/headsha + rm -f src/Settings.xml + calculate /tmp/headsha fi +if [ ! -f "$CACHEDIR/$DEV_SHA" ]; then + # Carry the same test harness across revisions, without copying game calculations. + git diff --binary "$DEV_SHA" -- .busted src/HeadlessWrapper.lua spec/ > /tmp/DevPatch + git reset --hard "$DEV_SHA" + git clean -fd + git apply --allow-empty --index /tmp/DevPatch + calculate "$CACHEDIR" + date > "$CACHEDIR/$DEV_SHA" + echo "[+] Base calculated: $DEV_SHA" +else + echo "[+] Base reused: $DEV_SHA" +fi +[ "${BASE_ONLY:-0}" != 1 ] || exit 0 -for runTime in "$CACHEDIR"/*.time -do - BASENAME=$(basename "$runTime") - - DIFFOUTPUT=$(luajit spec/DiffRuntime.lua "/tmp/headsha/$BASENAME" "$runTime" "$BASENAME") || { - echo "## Runtime comparison for $BASENAME" - echo '```' - echo "$DIFFOUTPUT" - echo '```' - } -done - -for build in "$CACHEDIR"/*.build -do - BASENAME=$(basename "$build") - - # Only print the header if there is a diff to display - DIFFOUTPUT=$(diff <(xmllint --exc-c14n "$build") <(xmllint --exc-c14n "/tmp/headsha/$BASENAME")) || { - echo "## Savefile Diff for $BASENAME" - echo '```diff' - echo "$DIFFOUTPUT" - echo '```' - } - - # Dedicated output diff - DIFFOUTPUT=$(luajit spec/DiffOutput.lua "/tmp/headsha/$BASENAME" "$build") || { - echo "## Output Diff for $BASENAME" - echo '```' - echo "$DIFFOUTPUT" - echo '```' - } +# Differences are findings. Missing files or a failed comparison are errors. +report() { + title=$1; language=$2; shift 2 + if output=$("$@"); then return; else status=$?; fi + [ "$status" -eq 1 ] && [ -n "$output" ] || return "$status" + printf '## %s\n```%s\n%s\n```\n' "$title" "$language" "$output" +} +compared=0 +for base in "$CACHEDIR"/*.build; do + name=$(basename "$base" .build) + report "Runtime comparison for $name.time" '' luajit spec/DiffRuntime.lua "/tmp/headsha/$name.time" "$CACHEDIR/$name.time" "$name.time" + xmllint --exc-c14n "$base" > /tmp/base.xml + xmllint --exc-c14n "/tmp/headsha/$name.build" > /tmp/head.xml + report "Savefile Diff for $name.build" diff diff /tmp/base.xml /tmp/head.xml + report "Output Diff for $name.build" '' luajit spec/DiffOutput.lua "/tmp/headsha/$name.build" "$base" + compared=$((compared + 1)) done +echo "[+] Compared $compared builds: $DEV_SHA -> $headsha" diff --git a/spec/GenerateBuilds.lua b/spec/GenerateBuilds.lua index d4cf5883929..2b4d76e03ef 100644 --- a/spec/GenerateBuilds.lua +++ b/spec/GenerateBuilds.lua @@ -10,91 +10,44 @@ function build:Save(xml) end end -local function fetchBuilds(path) - local lastDLtime = GetTime() - local co = coroutine.create(function(path) - if os.getenv("BUILDLINKS") then - local fileHnd, errMsg = io.open(os.getenv("BUILDLINKS"), "r") - if not fileHnd then error(errMsg) end - local fileText = fileHnd:read("*a") - fileHnd:close() - for line in splitLines(fileText) do - if line ~= "" then - local filename = line:gsub('%W', '') - -- Check cached XML before matching a download provider. - local fileHnd = io.open((os.getenv("CACHEDIR") or "/tmp") .. "/" .. filename .. ".xml", "r") - if fileHnd then - coroutine.yield({ xml = fileHnd:read("*a"), filename = filename, link = line }) - fileHnd:close() - else - for j = 1, #buildSites.websiteList do - if line:match(buildSites.websiteList[j].matchURL) then - -- Throttle build downloads to 15 per 10 seconds - local timeSinceLastDL = GetTime() - lastDLtime - if timeSinceLastDL < 666 then - posix.nanosleep(0, (666 - timeSinceLastDL) * 1000000) - end - buildSites.DownloadBuild(line, buildSites.websiteList[j], function(isSuccess, data) - lastDLtime = GetTime() - if isSuccess then - local xml = Inflate(common.base64.decode(data:gsub("-", "+"):gsub("_", "/"))) - local xmlHnd = io.open((os.getenv("CACHEDIR") or "/tmp") .. "/" .. filename .. ".xml", "w") - xmlHnd:write(xml) - xmlHnd:close() - coroutine.yield({ xml = xml, filename = filename, link = line }) - else - print("Failed to download build: " .. line) - end - end) - break - elseif j == #buildSites.websiteList then - print("Failed to match provider for: " .. line) - end - end - end - end - end - else - for file in lfs.dir(path) do - if file ~= "." and file ~= ".." then - local f = path .. '/' .. file - local attr = lfs.attributes(f) - assert(type(attr) == "table") - if attr.mode ~= "directory" and file:match("^.+(%..+)$") == ".xml" then - local fileHnd, errMsg = io.open(f, "r") - if not fileHnd then error(errMsg) end - local fileText = fileHnd:read("*a") - fileHnd:close() - coroutine.yield({ xml = fileText, filename = file }) - end - end - end - end - end) - return function() - local ok, result = coroutine.resume(co, path) - if not ok then error(result) end - return result - end +-- The API adapter supplies local XML files. A missing file is a failed test, +-- not a reason to try another download provider or silently skip the build. +local inputs = {} +if os.getenv("BUILDLINKS") then + local list = assert(io.open(os.getenv("BUILDLINKS"), "r")) + for name in list:lines() do + name = name:gsub("\r$", "") + assert(#name == 64 and name:match("^%x+$"), "Invalid corpus build name") + inputs[#inputs + 1] = { filename = name, path = assert(os.getenv("CACHEDIR")) .. "/" .. name .. ".xml" } + end + list:close() +else + for name in lfs.dir("../spec/TestBuilds") do + if name:match("%.xml$") then + inputs[#inputs + 1] = { filename = name, path = "../spec/TestBuilds/" .. name } + end + end end -for testBuild in fetchBuilds("../spec/TestBuilds") do - local filePath = (os.getenv("BUILDCACHEPREFIX") or "/tmp") .. "/" .. testBuild.filename - local startTime = GetTime() - - -- Compute the build - print("[+] Computing " .. filePath) - loadBuildFromXML(testBuild.xml) - local calcDuration = GetTime() - startTime - print("[-] Computed " .. filePath .. " in " .. calcDuration .. "ms") - - -- Save the computed build xml. Include full minion and player outputs. - local buildHnd = io.open(filePath .. ".build", "w+") - buildHnd:write(build:SaveDB("Cache")) - buildHnd:close() +for _, input in ipairs(inputs) do + local file = assert(io.open(input.path, "r")) + local xml = file:read("*a") + file:close() + local document, err = common.xml.ParseXML(xml) + assert(document and not err and document[1] and document[1].elem == "PathOfBuilding", "Invalid build XML: " .. input.filename) + local filePath = (os.getenv("BUILDCACHEPREFIX") or "/tmp") .. "/" .. input.filename + local startTime = GetTime() + print("[+] Computing " .. filePath) + loadBuildFromXML(xml) + assert(not build.abortSave and type(build.calcsTab.mainOutput.Life) == "number", "No calculated output: " .. input.filename) + local calcDuration = GetTime() - startTime + print("[-] Computed " .. filePath .. " in " .. calcDuration .. "ms") - -- Save the amount of time calculation of this build took - local timeHnd = io.open(filePath .. ".time", "w+") - timeHnd:write(calcDuration) - timeHnd:close() + local saved = assert(build:SaveDB("Cache"), "Could not save " .. input.filename) + local buildFile = assert(io.open(filePath .. ".build", "w")) + buildFile:write(saved) + buildFile:close() + local timeFile = assert(io.open(filePath .. ".time", "w")) + timeFile:write(calcDuration) + timeFile:close() end From 692055f64128ca315bab9d2d2b85065c48a96fce Mon Sep 17 00:00:00 2001 From: AdamZ Date: Thu, 10 Sep 2026 20:50:43 -0700 Subject: [PATCH 11/14] Allow the container user to read the downloaded corpus --- spec/BuildCache.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/BuildCache.sh b/spec/BuildCache.sh index df19e039c0a..f388b0ab657 100644 --- a/spec/BuildCache.sh +++ b/spec/BuildCache.sh @@ -4,6 +4,7 @@ if [ -z "${CORPUS_FILE:-}" ]; then CORPUS_FILE=$(mktemp) curl --fail --show-error --silent --max-time 60 \ https://api.pob.codes/test-builds/corpus -o "$CORPUS_FILE" + chmod a+r "$CORPUS_FILE" # The container reads this public feed as nobody. fi corpus_hash=$(sha256sum "$CORPUS_FILE" | cut -d ' ' -f 1) test_hash=$(git ls-files -z -- .busted docker-compose.yml src/HeadlessWrapper.lua \ From 3268fb4d2fd55c5ffc65ddb1d6c42989d7ba01d7 Mon Sep 17 00:00:00 2001 From: AdamZ Date: Thu, 10 Sep 2026 21:03:09 -0700 Subject: [PATCH 12/14] Stop the batch runner immediately after a calculation failure --- spec/BuildDiff.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/BuildDiff.sh b/spec/BuildDiff.sh index bd9fe62c6d2..5fc69d4a6e6 100644 --- a/spec/BuildDiff.sh +++ b/spec/BuildDiff.sh @@ -33,7 +33,7 @@ cp "$CACHEDIR/builds.txt" spec/builds.txt calculate() { export BUILDCACHEPREFIX="$1" mkdir -p "$BUILDCACHEPREFIX" - cat spec/builds.txt | dos2unix | parallel --jobs "${BUILD_JOBS:-2}" --halt soon,fail=1 --will-cite --ungroup --pipe -N50 \ + cat spec/builds.txt | dos2unix | parallel --jobs "${BUILD_JOBS:-2}" --halt now,fail=1 --will-cite --ungroup --pipe -N50 \ 'batch=$(mktemp); cat > "$batch"; BUILDLINKS="$batch" busted --lua=luajit -r generate' busted --lua=luajit -r generate expected=$(( $(wc -l < spec/builds.txt) + $(find spec/TestBuilds -maxdepth 1 -name '*.xml' | wc -l) )) From f308d044bc453b36516da10fdb7c94e3220fe64d Mon Sep 17 00:00:00 2001 From: AdamZ Date: Thu, 10 Sep 2026 21:03:29 -0700 Subject: [PATCH 13/14] Include the official runtime version in cache identity --- docker-compose.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index bff1d081a3d..cc6e46fa4ff 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -1,7 +1,7 @@ services: busted-tests: #build: . - image: ghcr.io/pathofbuildingcommunity/pathofbuilding-tests:latest + image: &test-image ghcr.io/pathofbuildingcommunity/pathofbuilding-tests@sha256:171dc3da232b8c874882e4ae3b3aa4a6e130a6c9450a31904b312435a6bf5daf environment: HOME: /tmp LUA_PATH: "/usr/share/lua/5.1/?.lua;/usr/share/lua/5.1/?/init.lua;;" @@ -30,7 +30,7 @@ services: - ./:/workdir:ro busted-diff: #build: . - image: ghcr.io/pathofbuildingcommunity/pathofbuilding-tests:latest + image: *test-image environment: #Where in the container the folders are stored WORKDIR: /workdir HOME: /tmp From 638da10d046fe2204dac49c699cc4ff201b6a5b7 Mon Sep 17 00:00:00 2001 From: AdamZ Date: Thu, 10 Sep 2026 22:30:57 -0700 Subject: [PATCH 14/14] Show build comparison summaries and fail on stat differences --- .github/workflows/buildtest.yml | 11 ++++- CONTRIBUTING.md | 2 +- spec/BuildDiff.sh | 7 +++- spec/BuildSummary.lua | 67 +++++++++++++++++++++++++++++++ spec/System/BuildSummary_spec.lua | 45 +++++++++++++++++++++ 5 files changed, 128 insertions(+), 4 deletions(-) create mode 100644 spec/BuildSummary.lua create mode 100644 spec/System/BuildSummary_spec.lua diff --git a/.github/workflows/buildtest.yml b/.github/workflows/buildtest.yml index 9037636e1a9..869f44a7deb 100644 --- a/.github/workflows/buildtest.yml +++ b/.github/workflows/buildtest.yml @@ -61,12 +61,15 @@ jobs: shell: bash run: | mkdir -p /tmp/pob-cache + result=0 docker compose run --rm -v /tmp/pob-cache:/cache \ -v "$CORPUS_FILE:/corpus.json:ro" -e CORPUS_FILE=/corpus.json \ - busted-diff | tee /tmp/dockerlog + busted-diff | tee /tmp/dockerlog || result=$? test -f "/tmp/pob-cache/$CACHE_KEY/$DEVREF" + echo "base_complete=true" >> "$GITHUB_OUTPUT" + exit "$result" - name: Save the completed base - if: success() && steps.cache.outputs.cache-hit != 'true' + if: ${{ !cancelled() && steps.calculate.outputs.base_complete == 'true' && steps.cache.outputs.cache-hit != 'true' }} uses: actions/cache/save@v4 continue-on-error: true with: @@ -79,6 +82,10 @@ jobs: if [ -f /tmp/dockerlog ]; then sed -n '/^## /,$p' /tmp/dockerlog > /tmp/build-diff-output [ -s /tmp/build-diff-output ] || rm -f /tmp/build-diff-output + sed -n '/^## Build comparison summary$/,$p' /tmp/dockerlog >> "$GITHUB_STEP_SUMMARY" + fi + if [ ! -s "$GITHUB_STEP_SUMMARY" ]; then + printf '## Build comparison summary\n\nCalculation or comparison did not finish. See the **Calculate builds** log for the error.\n' >> "$GITHUB_STEP_SUMMARY" fi - name: Upload comparison output if: always() && env.BASE_ONLY != '1' diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 87710a04ee2..a85475b9584 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -305,7 +305,7 @@ While both can be ran locally it's recommended to use the provided docker image PoB uses the [Busted](https://lunarmodules.github.io/busted/) framework to run its tests. Tests are stored under `spec/System` and run automatically when a PR is modified. More tests can be added to this folder to test specific functionality, or new test builds can be added to ensure nothing changed that wasn't intended. -To run the build difference tests, first fetch `origin/dev`, then run `docker compose run --rm busted-diff`. Set `DEVREF` to compare against another branch or commit. The test uses the same PoB Codes builds and checked-in fixtures on both revisions, and reports stat, saved-XML and timing differences. Base results are reused from the Docker `build-cache` volume when the base, corpus and test tools match; `docker compose down --volumes` clears that cache. GitHub Actions prepares the shared baseline nightly, while PRs calculate both sides themselves if it is unavailable. +To run the build difference tests, first fetch `origin/dev`, then run `docker compose run --rm busted-diff`. Set `DEVREF` to compare against another branch or commit. The test uses the same PoB Codes builds and checked-in fixtures on both revisions, and reports stat, saved-XML and timing differences. Calculated-stat differences fail the check and appear in the GitHub run summary; the full report remains in the log and `build-diff-output` artifact. Base results are reused from the Docker `build-cache` volume when the base, corpus and test tools match; `docker compose down --volumes` clears that cache. GitHub Actions prepares the shared baseline nightly, while PRs calculate both sides themselves if it is unavailable. 1. Install [Docker](https://www.docker.com/get-started) 2. Run `docker-compose up` from the command line diff --git a/spec/BuildDiff.sh b/spec/BuildDiff.sh index 5fc69d4a6e6..640361e0533 100644 --- a/spec/BuildDiff.sh +++ b/spec/BuildDiff.sh @@ -62,12 +62,16 @@ else fi [ "${BASE_ONLY:-0}" != 1 ] || exit 0 -# Differences are findings. Missing files or a failed comparison are errors. +# Keep the full report, collecting stat differences for the final summary/check. +: > /tmp/build-stat-diffs report() { title=$1; language=$2; shift 2 if output=$("$@"); then return; else status=$?; fi [ "$status" -eq 1 ] && [ -n "$output" ] || return "$status" printf '## %s\n```%s\n%s\n```\n' "$title" "$language" "$output" + case "$title" in + "Output Diff for "*) printf '## %s\n%s\n' "$title" "$output" >> /tmp/build-stat-diffs ;; + esac } compared=0 for base in "$CACHEDIR"/*.build; do @@ -80,3 +84,4 @@ for base in "$CACHEDIR"/*.build; do compared=$((compared + 1)) done echo "[+] Compared $compared builds: $DEV_SHA -> $headsha" +luajit spec/BuildSummary.lua /tmp/build-stat-diffs "$compared" "$DEV_SHA" "$headsha" diff --git a/spec/BuildSummary.lua b/spec/BuildSummary.lua new file mode 100644 index 00000000000..f45556cc13a --- /dev/null +++ b/spec/BuildSummary.lua @@ -0,0 +1,67 @@ +-- Summarize the existing stat report without changing which differences count. +local function summarize(lines, compared, baseRef, headRef) + local builds, stats, build = {}, {}, nil + local changedBuilds, changedValues = 0, 0 + for line in lines do + build = line:match("^## Output Diff for (.+)%.build$") or build + local stat, actor = line:match("^(.-) Mismatch in (%a+) outputs:") + if stat then + assert(build, "Stat difference has no build name") + local head = assert(lines():match("^%s*head Output: (.*)$"), "Missing head value") + local base = assert(lines():match("^%s*dev Output: (.*)$"), "Missing base value") + if not builds[build] then + builds[build] = true + changedBuilds = changedBuilds + 1 + end + local key = actor .. "." .. stat + local entry = stats[key] or { key = key, count = 0, build = build, base = base, head = head } + entry.count = entry.count + 1 + stats[key] = entry + changedValues = changedValues + 1 + end + end + local function escape(value) + return tostring(value):gsub("&", "&"):gsub("<", "<"):gsub(">", ">"):gsub("|", "|"):gsub("`", "`") + end + local rows = {} + for _, entry in pairs(stats) do rows[#rows + 1] = entry end + -- Put common player results first; this affects presentation only. + local priority = { ["player.Life"] = 1, ["player.Mana"] = 2, ["player.EnergyShield"] = 3, + ["player.TotalDPS"] = 4, ["player.CombinedDPS"] = 5, ["player.FullDPS"] = 6 } + table.sort(rows, function(a, b) + local ap, bp = priority[a.key] or 7, priority[b.key] or 7 + if ap ~= bp then return ap < bp end + if a.count ~= b.count then return a.count > b.count end + return a.key < b.key + end) + local output = { + "## Build comparison summary", "", + changedBuilds > 0 and "**Changes found: review required.**" or "**No calculated-stat differences found.**", "", + string.format("Compared **%d builds**. **%d builds** have calculated-stat differences (**%d changed values**).", compared, changedBuilds, changedValues), "", + "Base: `" .. escape(baseRef) .. "`. PR: `" .. escape(headRef) .. "`.", "", + } + if #rows > 0 then + output[#output + 1] = string.format("Showing %d of %d changed stats, with one example for each. Common player results appear first; other stats are ordered by affected build count.", math.min(25, #rows), #rows) + output[#output + 1] = "" + output[#output + 1] = "| Stat | Affected builds | Example build | Before | After |" + output[#output + 1] = "| --- | ---: | --- | ---: | ---: |" + for index = 1, math.min(25, #rows) do + local entry = rows[index] + local name = entry.build + if #name == 64 and name:match("^%x+$") then name = name:sub(1, 12) end + output[#output + 1] = string.format("| %s | %d | %s | %s | %s |", escape(entry.key), entry.count, escape(name), escape(entry.base), escape(entry.head)) + end + output[#output + 1] = "" + end + output[#output + 1] = "Calculated-stat differences fail this check and may be intentional or unintended. Saved-XML and timing differences remain informational." + output[#output + 1] = "" + output[#output + 1] = "All differences are in the calculation log and the **build-diff-output** artifact. API build identifiers above are shortened to 12 characters for searching the full report." + return table.concat(output, "\n"), changedBuilds +end + +if arg and arg[0]:match("BuildSummary%.lua$") then + local output, changed = summarize(io.lines(assert(arg[1])), assert(tonumber(arg[2])), assert(arg[3]), assert(arg[4])) + print(output) + os.exit(changed > 0 and 1 or 0) +end +return summarize diff --git a/spec/System/BuildSummary_spec.lua b/spec/System/BuildSummary_spec.lua new file mode 100644 index 00000000000..957f3fbd422 --- /dev/null +++ b/spec/System/BuildSummary_spec.lua @@ -0,0 +1,45 @@ +describe("Build comparison summary", function() + local summarize = dofile("../spec/BuildSummary.lua") + local function lines(text) + return (text .. "\n"):gmatch("([^\n]*)\n") + end + it("reports identical calculations without a mismatch", function() + local output, changed = summarize(lines(""), 5, "base", "head") + assert.equals(0, changed) + assert.matches("No calculated-stat differences found", output, 1, true) + assert.matches("Compared **5 builds**", output, 1, true) + end) + it("counts affected builds once and keeps player, minion and missing values", function() + local output, changed = summarize(lines([[## Output Diff for A.build +Life Mismatch in player outputs: + head Output: 118 + dev Output: 92 +Life Mismatch in minion outputs: + head Output: 20 + dev Output: nil +## Output Diff for B.build +Life Mismatch in player outputs: + head Output: 200 + dev Output: 100]]), 5, "base", "head") + assert.equals(2, changed) + assert.matches("3 changed values", output, 1, true) + assert.matches("| player.Life | 2 | A | 92 | 118 |", output, 1, true) + assert.matches("| minion.Life | 1 | A | nil | 20 |", output, 1, true) + end) + it("limits display rows without dropping mismatches and escapes table content", function() + local input = { "## Output Diff for A|.build" } + for index = 1, 30 do + input[#input + 1] = string.format("Stat%d Mismatch in player outputs:\n\thead Output: 2\n\tdev Output: 1", index) + end + local output, changed = summarize(lines(table.concat(input, "\n")), 1, "base", "head") + assert.equals(1, changed) + assert.matches("Showing 25 of 30 changed stats", output, 1, true) + assert.matches("30 changed values", output, 1, true) + assert.matches("A|<B>", output, 1, true) + end) + it("rejects a truncated difference instead of reporting a clean comparison", function() + assert.has_error(function() + summarize(lines("## Output Diff for A.build\nLife Mismatch in player outputs:\n"), 1, "base", "head") + end) + end) +end)