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 6ee6ad43231..869f44a7deb 100644 --- a/.github/workflows/buildtest.yml +++ b/.github/workflows/buildtest.yml @@ -1,93 +1,96 @@ ---- -name: Run Tests +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 - 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 - 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 - path: /tmp/cache/ - if_no_artifact_found: warn - search_artifacts: true - - name: Calculate build xmls and differences between them + 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: | - 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 - 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 - uses: actions/upload-artifact@v4 + 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: 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' }} + 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 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 + 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 || result=$? + test -f "/tmp/pob-cache/$CACHE_KEY/$DEVREF" + echo "base_complete=true" >> "$GITHUB_OUTPUT" + exit "$result" + - name: Save the completed base + if: ${{ !cancelled() && steps.calculate.outputs.base_complete == 'true' && steps.cache.outputs.cache-hit != 'true' }} + uses: actions/cache/save@v4 + continue-on-error: true with: - name: build-xmls - path: './new-build-xmls/*' - - name: Upload dev ref cache - if: ${{ steps.download-dev-ref-cache.outputs.found_artifact == 'false' }} + 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: | + 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' uses: actions/upload-artifact@v4 with: - name: cache-devref-${{ steps.get-dev-ref.outputs.devref }} - path: /tmp/cache/ + name: build-diff-output + path: /tmp/build-diff-output + if-no-files-found: ignore 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..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. -### Running tests +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/docker-compose.yml b/docker-compose.yml index b01cfb360ab..cc6e46fa4ff 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -1,9 +1,24 @@ 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;;" + 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 -R nobody:nobody "$$CACHEDIR" + fi + exec su -p nobody -s /bin/sh -c "$$*" + - -- container_name: pathofbuilding-tests command: busted --lua=luajit security_opt: @@ -13,3 +28,27 @@ services: working_dir: /workdir volumes: - ./:/workdir:ro + busted-diff: + #build: . + image: *test-image + environment: #Where in the container the folders are stored + 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;;" + LUA_CPATH: "/usr/lib/lua/5.1/?.so;;" + container_name: busted-diff + 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: diff --git a/spec/BuildCache.sh b/spec/BuildCache.sh new file mode 100644 index 00000000000..f388b0ab657 --- /dev/null +++ b/spec/BuildCache.sh @@ -0,0 +1,14 @@ +# 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" + 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 \ + 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 180960a11d1..640361e0533 100644 --- a/spec/BuildDiff.sh +++ b/spec/BuildDiff.sh @@ -1,73 +1,87 @@ #!/bin/sh +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 /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") - -rm -rf /tmp/headsha && mkdir /tmp/headsha -rm /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 - - # 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 && \ - 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 $? +# 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 -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") +# 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 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) )) + 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" +} - # 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 '```' - } +# 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 - # Dedicated output diff - DIFFOUTPUT=$(luajit spec/DiffOutput.lua "/tmp/headsha/$BASENAME" "$build") || { - echo "## Output Diff for $BASENAME" - echo '```' - echo "$DIFFOUTPUT" - echo '```' - } +# 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 + 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" +luajit spec/BuildSummary.lua /tmp/build-stat-diffs "$compared" "$DEV_SHA" "$headsha" 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/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/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..2b4d76e03ef 100644 --- a/spec/GenerateBuilds.lua +++ b/spec/GenerateBuilds.lua @@ -1,89 +1,53 @@ -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 - 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 - -- 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) - end - break - elseif j == #buildSites.websiteList then - print("Failed to match provider for: " .. line) - 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 +-- 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 -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") +-- 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 - -- 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: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 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) 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) 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 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 }, diff --git a/src/HeadlessWrapper.lua b/src/HeadlessWrapper.lua index 5f246c4a8e3..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 @@ -59,7 +39,6 @@ function GetVirtualScreenSize() return 1920, 1080 end -<<<<<<< HEAD posix = require("posix") -- Search Handles @@ -147,9 +126,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 = "" @@ -165,7 +141,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)