diff --git a/.claude/skills/fix-issue/references/verification-apple.md b/.claude/skills/fix-issue/references/verification-apple.md index 3c7159c77a..46b845d38c 100644 --- a/.claude/skills/fix-issue/references/verification-apple.md +++ b/.claude/skills/fix-issue/references/verification-apple.md @@ -42,7 +42,7 @@ A raw `xcodebuild` failure comes back as a head-and-tail excerpt of about 10,000 - **The accessibility tree differs between this machine and the CI runner.** A contextual `NSMenu` has its identifier locally and not on CI, so reach it with `app.windows.firstMatch.menus.firstMatch`. The data grid is identifier `data-grid`, since the window holds several tables. Click the grid at an offset from `data-grid`, never on a row or cell element, which XCUITest reads as obscured. - **`.accessibilityIdentifier` on a SwiftUI container replaces the identifier of every child** in that hosting tree. When the container needs one, put `.accessibilityElement(children: .contain)` before it. - **Diagnose by dumping the tree** (`print(app.windows.firstMatch.debugDescription)` inside a `UITestCase`), not by guessing. -- **When UI tests cannot start on this machine** (`Timed out while enabling automation mode`, zero cases), reproduce it on an untouched suite, then say in the PR that CI runs the suite. When a test fails only on the runner for a reason you cannot establish, quarantine it in `.github/macos-ui-test-quarantine.txt` with the reason instead of guessing at a fix. +- **When UI tests cannot start on this machine** (`Timed out while enabling automation mode`, zero cases), reproduce it on an untouched suite, then say in the PR that CI runs the suite. When a test fails only on the runner for a reason you cannot establish, mark it XCTExpectFailure with the reason and an issue link instead of guessing at a fix. ## Lint diff --git a/.claude/skills/fix-issue/scripts/lib/platform-apple.sh b/.claude/skills/fix-issue/scripts/lib/platform-apple.sh index 2a3d96404c..9e7fe6b7fa 100644 --- a/.claude/skills/fix-issue/scripts/lib/platform-apple.sh +++ b/.claude/skills/fix-issue/scripts/lib/platform-apple.sh @@ -5,22 +5,22 @@ # verify.sh [options] generate # verify.sh [options] build [Scheme] # default scheme: TablePro # verify.sh [options] plugins # AllPlugins aggregate -# verify.sh [options] test [Suite…] -# verify.sh [options] uitest [Suite…] -# verify.sh [options] package [filter] # swift test in Packages/ -# verify.sh [options] ios [Suite…] # TableProMobileTests on a simulator +# verify.sh [options] test [Test...] # TestPlans/Unit.xctestplan, all of it when no Test is named +# verify.sh [options] uitest [Test...] # TestPlans/UI.xctestplan, all of it when no Test is named +# verify.sh [options] package [regex] # swift test in Packages/ +# verify.sh [options] ios [Suite...] # TableProMobileTests on a simulator # verify.sh [options] abi -# verify.sh [options] lint [path…] +# verify.sh [options] lint [path...] # verify.sh [options] docs [docs-checkout] # TableProApp/docs checks against this tree (default ../tablepro-docs) -# verify.sh [options] l10n # plugin and package strings in the app catalog, managed manually +# verify.sh [options] l10n # plugin and package strings in the mac and ios catalogs # +# A Test is Suite, Suite/case(), or either behind its target name (TableProGrammarsTests/Suite). +# A bare Suite is a Swift type name, looked up in every test target of the tree. +# A test target no plan lists (TableProCore, TableProOracle) runs under swift test, not xcodebuild. +# `test` with no Test runs the Unit plan only; those two packages run with `package `. +# A name no target declares exits 3 before anything builds. A Test that runs nothing is FAIL. # Here --no-wait skips the wait for another xcodebuild, and --offline pins SwiftPM to Package.resolved. -# Sourced by verify.sh, which owns the options, the log and the verdict. A backend defines -# platform_step, which runs one step and ends in emit, and platform_parse, which re-derives a -# verdict from a stored log that has no receipt. STEP_DETAIL is read by emit over there, which is -# all SC2034 is off for. - MAX_WAIT_SECONDS=1800 # Known false failures in this checkout. Each one presents as a code defect and is not one. @@ -31,7 +31,7 @@ diagnose_environment() { found=0 fi if grep -q 'produced no further output' "$log" 2> /dev/null; then - note "cause: the Swift frontend crashed without a diagnostic. Flaky on this machine. Rerun before believing any 'cannot find type' error." + note "cause: the Swift frontend crashed without a diagnostic. Rerun before believing any 'cannot find type' error." found=0 fi if grep -q "Could not resolve package dependencies\|Couldn't fetch updates from remote" "$log" 2> /dev/null; then @@ -43,33 +43,33 @@ diagnose_environment() { found=0 fi if grep -q 'The test runner hung before establishing connection' "$log" 2> /dev/null; then - note "cause: the XCTest host wedged before running anything. Check the executed count below." + note "cause: the XCTest host wedged before running anything." + found=0 + fi + if grep -q 'Timed out while enabling automation mode\|failed to initialize for UI testing' "$log" 2> /dev/null; then + note "cause: Automation Mode is off and XCTest could not turn it on. Check with automationmodetool; enabling it needs an admin password." found=0 fi return $found } report_errors() { - local log="$1" - local errors + local log="$1" errors total errors="$(grep -E '(^|[^a-zA-Z])error: ' "$log" 2> /dev/null \ | sed 's/^.*\/\([^/]*\.swift:[0-9]*:[0-9]*\)/\1/' \ | sort -u | head -12)" - if [ -n "$errors" ]; then - echo "errors:" - printf '%s\n' "$errors" | sed 's/^/ /' - local total - total="$(grep -cE '(^|[^a-zA-Z])error: ' "$log" 2> /dev/null)" - [ "$total" -gt 12 ] && echo " … $((total - 12)) more, see the log" - fi + [ -n "$errors" ] || return 0 + echo "errors:" + printf '%s\n' "$errors" | sed 's/^/ /' + total="$(grep -cE '(^|[^a-zA-Z])error: ' "$log" 2> /dev/null)" + [ "$total" -gt 12 ] && echo " ... $((total - 12)) more, see the log" } # ---------------------------------------------------------------------------- environment setup_toolchain() { [ -x "${DEVELOPER_DIR:-/nonexistent}/usr/bin/xcodebuild" ] && return 0 - # The Xcode that xcode-select names wins when it is a full Xcode. Command Line Tools has no - # xcodebuild and no sourcekitd, so fall back to whichever Xcode is installed. + # Command Line Tools has no xcodebuild and no sourcekitd, so fall back to an installed Xcode. local candidate for candidate in "$(xcode-select -p 2> /dev/null)" \ /Applications/Xcode.app/Contents/Developer \ @@ -81,9 +81,7 @@ setup_toolchain() { done } -# Only a build of THIS checkout contends: every worktree builds into its own DerivedData, so a -# machine-wide wait stalled a worktree run for as long as a peer session built somewhere else. -# A build belongs here when it names this checkout's project or runs from its root. +# Every worktree builds into its own DerivedData, so only a build of this checkout contends. checkout_busy() { local pid for pid in $(pgrep -f 'Developer/usr/bin/xcodebuild' 2> /dev/null); do @@ -114,110 +112,317 @@ xcodebuild_flags() { is_offline && printf ' %s' "-disableAutomaticPackageResolution -onlyUsePackageVersionsFromResolvedFile" } -# ---------------------------------------------------------------------------- test reporting - -# Mutes failing cases listed in the CI quarantine files, so a red run reports only the failures -# that are new. -PASS_PATTERN="^test case .* passed|✔ test .* passed" -FAIL_PATTERN="^test case .* failed|✘ test .* (failed|recorded an issue)" - -report_tests() { - local log="$1" - local passed failed executed - passed="$(grep -ciE "$PASS_PATTERN" "$log" 2> /dev/null)" - failed="$(grep -ciE "$FAIL_PATTERN" "$log" 2> /dev/null)" - executed=$((passed + failed)) - note "cases: $executed executed, $passed passed, $failed failed" - - if [ "$executed" -eq 0 ]; then - STATUS=INCONCLUSIVE - note "cause: zero cases executed. The filter matched nothing, or the host wedged. A -only-testing filter naming a Swift Testing @Test function silently matches nothing and still prints TEST SUCCEEDED. Filter by suite." - return - fi - if [ "$failed" -eq 0 ]; then - # A crash, a timeout or an unrecognised line format fails the run without a case line the - # patterns above can count. This printed PASS over a log that said TEST FAILED. - if grep -qE '^\*\* TEST FAILED \*\*|^Failing tests:' "$log" 2> /dev/null; then - STATUS=FAIL - note "cause: xcodebuild reported a failure that no counted case line names. Read the block below." - note "$(grep -A 8 '^Failing tests:' "$log" 2> /dev/null | sed 's/^/ /' | head -10)" - return +# ---------------------------------------------------------------------------- test selection + +# One line per test target, "": the plan that lists it (Unit, UI), or +# swift: for a package test target no plan lists. +test_targets_table() { + local plan manifest package target table="" + for plan in Unit UI; do + [ -f "$REPO_ROOT/TestPlans/$plan.xctestplan" ] || continue + table="$table$(jq -r --arg plan "$plan" '.testTargets[].target.name + "\t" + $plan' \ + "$REPO_ROOT/TestPlans/$plan.xctestplan")"$'\n' + done + for manifest in "$REPO_ROOT"/Packages/*/Package.swift; do + [ -f "$manifest" ] || continue + package="${manifest%/Package.swift}" + package="${package##*/}" + for target in $(awk '/\.testTarget\(/ { t = 1 } + t && /name:/ { if (match($0, /"[^"]+"/)) print substr($0, RSTART + 1, RLENGTH - 2); t = 0 }' "$manifest"); do + case $'\n'"$table" in + *$'\n'"$target"$'\t'*) ;; + *) table="$table$target"$'\t'"swift:$package"$'\n' ;; + esac + done + done + printf '%s' "$table" +} + +route_of() { + printf '%s' "$TEST_TARGETS" | awk -F'\t' -v t="$1" '$1 == t { print $2; exit }' +} + +# The test targets that declare a type named $1, one per line. A file outside every known target +# prints as ?. +targets_declaring() { + local file part target + git -C "$REPO_ROOT" grep --untracked -l -E \ + "^[^/\"]*(struct|class|enum|actor|extension)[[:space:]]+$1([^A-Za-z0-9_]|\$)" -- '*Tests/*.swift' 2> /dev/null \ + | while IFS= read -r file; do + target="" + IFS=/ read -r -a parts <<< "$file" + for part in "${parts[@]}"; do + [ -n "$(route_of "$part")" ] && { target="$part"; break; } + done + printf '%s\n' "${target:-?${file%/*}}" + done | sort -u +} + +# Sets TARGET, TEST_PATH and ROUTE for one Test argument, or says why it cannot and fails. +resolve_test() { + local first="${1%%/*}" rest="" found known + [ "$first" = "$1" ] || rest="${1#*/}" + case "$first" in + '' | *[!A-Za-z0-9_]*) echo "not a test name: $1" >&2; return 1 ;; + esac + TARGET="" TEST_PATH="$rest" + # TableProGrammarsTests/case() names a suite that shares its target's name, not a target. + if [ -n "$(route_of "$first")" ] \ + && ! { [ -n "$rest" ] && [ "${rest#*/}" = "$rest" ] && [ "${rest%)}" != "$rest" ]; }; then + TARGET="$first" + else + found="$(targets_declaring "$first")" + known="$(grep -v '^?' <<< "$found")" + if [ -z "$found" ]; then + echo "no test type named $first in this tree" >&2 + return 1 + elif [ -z "$known" ]; then + echo "$first is declared only under ${found#?}, which no test plan or package lists" >&2 + case "$found" in *TableProMobileTests*) echo " run it with: verify.sh ios $first" >&2 ;; esac + return 1 + elif [ "$(wc -l <<< "$known" | tr -d ' ')" -gt 1 ]; then + echo "$first is declared in more than one test target; name it as /$1:" >&2 + sed 's/^/ /' <<< "$known" >&2 + return 1 fi - STATUS=PASS - return + TARGET="$known" TEST_PATH="$1" fi + ROUTE="$(route_of "$TARGET")" +} - local quarantine="$REPO_ROOT/.github/macos-test-quarantine.txt" - local ui_quarantine="$REPO_ROOT/.github/macos-ui-test-quarantine.txt" - - # Both quarantine files hold one case per line as Suite/case(), never a bare suite name, so a - # failing line is matched by its own case id. Matching the suite instead did two wrong things - # at once: it never matched the unit file at all, so every quarantined case was reported as a - # new failure; and on the UI file it muted the whole suite, hiding a real regression sitting - # beside a quarantined case. - # xcodebuild prints a failing case in three spellings and the quarantine files use only the - # last one, so every line is normalised to Suite/case() before the lookup: - # Test Case '-[TableProUITests.FooUITests testBar]' failed (1.0 seconds). XCTest - # Test case 'FooUITests.testBar()' failed on 'My Mac' ... XCTest - # Test case 'FooTests/bar()' failed on 'My Mac' ... Swift Testing - # Matching only the third muted nothing written by XCTest, which is every entry in the UI - # quarantine file, so a clean uitest run reported both quarantined cases as new failures. - local fail_lines unmuted muted_cases=0 - fail_lines="$(grep -iE "$FAIL_PATTERN" "$log" 2> /dev/null)" - unmuted="" - while IFS= read -r line; do - [ -n "$line" ] || continue - local case_id - case_id="$(printf '%s\n' "$line" | sed -E \ - -e "s|.*'-\[[A-Za-z0-9_]+\.([A-Za-z0-9_]+) ([A-Za-z0-9_]+)\]'.*|\1/\2()|" \ - -e "s|.*'([A-Za-z0-9_]+)\.([A-Za-z0-9_]+)\(\)'.*|\1/\2()|" \ - -e "s|.*'([A-Za-z0-9_]+/[A-Za-z0-9_]+\(\))'.*|\1|")" - # A line none of the three matched comes back unchanged, so require the normalised shape - # before trusting it. Failing to extract must report the line, never mute it. - case "$case_id" in - *' '* | *'/'*'/'*) case_id="" ;; - */*'()') ;; - *) case_id="" ;; - esac - if [ -n "$case_id" ] \ - && { grep -qxF "$case_id" "$quarantine" 2> /dev/null \ - || grep -qxF "$case_id" "$ui_quarantine" 2> /dev/null; }; then - muted_cases=$((muted_cases + 1)) - continue - fi - unmuted="$unmuted$line"$'\n' - done <<< "$fail_lines" - - [ "$muted_cases" -gt 0 ] && note "muted: $muted_cases quarantined case(s)" - - local suites - suites="$(printf '%s\n' "$unmuted" \ - | grep -oE "[A-Za-z_][A-Za-z0-9_]*Tests" \ - | grep -vxE "TableProTests|TableProUITests" \ - | sort -u)" - - if [ -z "$suites" ]; then - if [ -z "${unmuted//[$'\n' ]/}" ]; then - STATUS=PASS - note "verdict: no unexplained failures. Every failing case is quarantined." - return - fi - STATUS=FAIL - note "failing cases (no suite name on the line, read the log):" - note "$(printf '%s\n' "$unmuted" | sed 's/^/ /' | head -10)" - return +ere_escape() { + printf '%s' "$1" | sed 's/[][\\.*^$+?(){}|]/\\&/g' +} + +# Matches a test identifier, /(): sep is / in a result bundle and . under +# swift test. XCTest names a case without the parentheses, so they are optional. +test_regex() { + local target="$1" path="$2" sep="$3" + if [ -z "$path" ]; then + printf '^%s(%s|$)' "$(ere_escape "$target")" "$(ere_escape "$sep")" + else + printf '^%s%s%s(\\(\\))?(/|$)' "$(ere_escape "$target")" "$(ere_escape "$sep")" "$(ere_escape "${path%()}")" + fi +} + +# ---------------------------------------------------------------------------- test results + +# Every test case in a result bundle as "//retried?". A runner +# that crashed or never started is reported as a case under "System Failures"; it is not a test. +xcresult_cases() { + xcrun xcresulttool get test-results tests --path "$1" --compact 2> /dev/null | jq -r ' + [.. | objects | select((.nodeType // "") | endswith("test bundle"))][] | .name as $target + | .children[]? | (.nodeType == "Test Suite" and .name == "System Failures") as $system + | .. | objects | select(.nodeType == "Test Case") + | [ "\($target)/\(.nodeIdentifier // .name)", (if $system then "System Failure" else .result end), + (if .result == "Passed" and any(.children[]?; .nodeType == "Repetition" and .result == "Failed") + then "retried" else "" end) ] | @tsv' +} + +# Every test case in swift test's xunit files as "./". +xunit_cases() { + local file + for file in "$@"; do + [ -f "$file" ] || continue + xmllint --format "$file" 2> /dev/null | awk ' + function attr(line, name, v) { + if (!match(line, " " name "=\"[^\"]*\"")) return "" + v = substr(line, RSTART + length(name) + 3, RLENGTH - length(name) - 4) + gsub(/"/, "\"", v); gsub(/'/, "\047", v) + gsub(/</, "<", v); gsub(/>/, ">", v); gsub(/&/, "\\&", v) + return v + } + function flush() { if (open) print key "\t" state; open = 0 } + /]/ { + flush() + # A nested Swift Testing suite comes as Target.Outer.Inner; its filters use Outer/Inner. + cls = attr($0, "classname"); dot = index(cls, ".") + if (dot) { rest = substr(cls, dot + 1); gsub(/\./, "/", rest); cls = substr(cls, 1, dot) rest } + key = cls "/" attr($0, "name"); state = "Passed"; open = 1 + if ($0 ~ /\/>[[:space:]]*$/) flush() + next + } + open && /\/]/ { state = "Failed" } + /<\/testcase>/ { flush() } + END { flush() }' + done +} + +# Tallies a case list against the run's filters ("