refactor: deduplicate test.py logic, fold no-metadata into TEST_CASES, fix sendfile metadata bug

test.py

@@ -14,7 +14,7 @@ DEFAULT_SOURCE_DIR = os.path.join(TEST_DIR, "source")
 DEFAULT_DEST_DIR = os.path.join(TEST_DIR, "dest")
 
 SERVER_CMD = ["./build/server"]
-base_client_cmd = ["./build/client"]
+BASE_CLIENT_CMD = ["./build/client"]
 
 DISK_DEVICE = "/dev/nvme0n1p5"
 READ_BPS_MAX = "15M"
@@ -47,8 +47,11 @@ CLIENT_CMD_PREFIX = [
     f"IOWriteBandwidthMax={DISK_DEVICE} {WRITE_BPS_MAX}",
 ]
 
+BASE_CLIENT_FLAGS = ["--save-to-disk"]
+
 TEST_CASES = [
     {"name": "Standard", "flags": []},
+    {"name": "Standard (no metadata)", "flags": [], "use_metadata": False},
     {"name": "Multithreading (-m)", "flags": ["-m"]},
     {"name": "Compression (-c)", "flags": ["-c"]},
     {"name": "Chunk Serialization (-s)", "flags": ["-s"]},
@@ -157,204 +160,100 @@ def verify_transfer(source_dir, received_dir):
     return mismatches, missing
 
 
-def run_profile(profile_name, source_dir, dest_dir):
+def run_single_test(cmd, name, source_dir, dest_dir, *, source_prefix=None):
-    results = []
-    is_limited = profile_name != "Unlimited"
-    params = NETWORK_PROFILES[profile_name]
-    print(f"\n{'=' * 60}")
-    print(f"Profile: {profile_name}")
-    print(f"{'=' * 60}")
-
-    if is_limited:
-        p = params
-        print(f"  Network: rate={p['rate']}, delay={p['delay']} ±{p['jitter']}, loss={p['loss']}")
-        print(f"  Disk I/O: Reads <= {READ_BPS_MAX}, Writes <= {WRITE_BPS_MAX}")
-        client_prefix = CLIENT_CMD_PREFIX
-    else:
-        print("  No limits applied")
-        client_prefix = []
-
-    try:
-        if is_limited:
-            netem_apply(profile_name)
-        else:
-            netem_reset()
-
-        for case in TEST_CASES:
-            name = case["name"]
-            flags = case["flags"]
-            print(f"\n  --- {name} ---")
-
     if os.path.exists(dest_dir):
         shutil.rmtree(dest_dir)
 
-            server_process = None
+    server = subprocess.Popen(SERVER_CMD, stdout=subprocess.DEVNULL, stderr=None)
-            try:
-                server_process = subprocess.Popen(
-                    SERVER_CMD, stdout=subprocess.DEVNULL, stderr=None
-                )
     time.sleep(0.5)
 
-                env = os.environ.copy()
-
-                client_cmd = (
-                    client_prefix
-                    + base_client_cmd
-                    + ["--source-dir", source_dir, "--dest-dir", dest_dir, "--save-to-disk", "-M"]
-                    + flags
-                )
-                print(f"    Running: {' '.join(client_cmd)}")
-
-                start_time = time.monotonic()
-                client_result = subprocess.run(
-                    client_cmd, env=env, text=True, capture_output=True
-                )
-                end_time = time.monotonic()
-                duration = end_time - start_time
-
-                if server_process:
     try:
-                        server_process.wait(timeout=5)
+        start = time.monotonic()
+        result = subprocess.run(cmd, text=True, capture_output=True)
+        duration = time.monotonic() - start
+    finally:
+        try:
+            server.wait(timeout=5)
         except subprocess.TimeoutExpired:
-                        server_process.kill()
+            server.kill()
-                        server_process.wait()
+            server.wait()
-                    server_process = None
 
     mismatches, missing = [], []
-                if client_result.returncode == 0:
+    if result.returncode == 0:
+        if source_prefix is not None:
+            received = os.path.join(dest_dir, source_prefix)
+        else:
             received = os.path.join(dest_dir, os.path.abspath(source_dir).lstrip(os.sep))
         mismatches, missing = verify_transfer(source_dir, received)
 
     entry = {
         "name": name,
-                    "suite": profile_name,
+        "time": f"{duration:.4f}s" if result.returncode == 0 else "N/A",
-                    "time": f"{duration:.4f}s" if client_result.returncode == 0 else "N/A",
     }
 
-                if client_result.returncode == 0 and not mismatches and not missing:
+    if result.returncode == 0 and not mismatches and not missing:
         entry["status"] = "Success"
         entry["error"] = ""
     else:
         entry["status"] = "Failed"
         errors = []
-                    if client_result.returncode != 0:
+        if result.returncode != 0:
             err = (
-                            client_result.stderr.strip().split("\n")[0]
+                result.stderr.strip().split("\n")[0]
-                            if client_result.stderr
+                if result.stderr
                 else (
-                                client_result.stdout.strip().split("\n")[0]
+                    result.stdout.strip().split("\n")[0]
-                                if client_result.stdout
+                    if result.stdout
                     else "No output"
                 )
             )
-                        errors.append(f"Exit code {client_result.returncode}: {err[:80]}")
+            errors.append(f"Exit code {result.returncode}: {err[:80]}")
         if missing:
             errors.append(f"Missing ({len(missing)}): {', '.join(missing[:5])}")
         if mismatches:
             errors.append(f"Mismatch ({len(mismatches)}): {', '.join(mismatches[:3])}")
         entry["error"] = " | ".join(errors)
 
-                results.append(entry)
+    return entry
 
-            except subprocess.TimeoutExpired:
-                results.append(
-                    {"name": name, "suite": profile_name, "status": "Timeout", "time": "N/A", "error": "Exceeded 15s"}
-                )
-            except Exception as e:
-                results.append(
-                    {"name": name, "suite": profile_name, "status": "Error", "time": "N/A", "error": str(e)}
-                )
-            finally:
-                if server_process:
-                    try:
-                        server_process.wait(timeout=5)
-                    except subprocess.TimeoutExpired:
-                        server_process.kill()
-                        server_process.wait()
 
-        # Run one case without metadata transfer to verify -M disabled works
+def run_client_tests(profile_name, source_dir, dest_dir, client_prefix):
-        print(f"\n  --- Standard (no metadata) ---")
+    results = []
-        if os.path.exists(dest_dir):
+    for case in TEST_CASES:
-            shutil.rmtree(dest_dir)
+        name = case["name"]
-        server_process2 = None
+        flags = list(BASE_CLIENT_FLAGS)
-        try:
+        if case.get("use_metadata", True):
-            server_process2 = subprocess.Popen(
+            flags.append("-M")
-                SERVER_CMD, stdout=subprocess.DEVNULL, stderr=None
+        flags += case["flags"]
-            )
-            time.sleep(0.5)
 
-            env = os.environ.copy()
+        cmd = (
-            client_cmd = (
             client_prefix
-                + base_client_cmd
+            + BASE_CLIENT_CMD
-                + ["--source-dir", source_dir, "--dest-dir", dest_dir, "--save-to-disk"]
+            + ["--source-dir", source_dir, "--dest-dir", dest_dir]
+            + flags
         )
-            print(f"    Running: {' '.join(client_cmd)}")
 
-            start_time = time.monotonic()
+        print(f"\n  --- {name} ---")
-            client_result = subprocess.run(
+        print(f"    Running: {' '.join(cmd)}")
-                client_cmd, env=env, text=True, capture_output=True
-            )
-            end_time = time.monotonic()
-            duration = end_time - start_time
 
-            if server_process2:
         try:
-                    server_process2.wait(timeout=5)
+            result = run_single_test(cmd, name, source_dir, dest_dir)
-                except subprocess.TimeoutExpired:
+            result["suite"] = profile_name
-                    server_process2.kill()
+            results.append(result)
-                    server_process2.wait()
-                server_process2 = None
-
-            mismatches, missing = [], []
-            if client_result.returncode == 0:
-                received = os.path.join(dest_dir, os.path.abspath(source_dir).lstrip(os.sep))
-                mismatches, missing = verify_transfer(source_dir, received)
-
-            entry = {
-                "name": "Standard (no metadata)",
-                "suite": profile_name,
-                "time": f"{duration:.4f}s" if client_result.returncode == 0 else "N/A",
-            }
-
-            if client_result.returncode == 0 and not mismatches and not missing:
-                entry["status"] = "Success"
-                entry["error"] = ""
-            else:
-                entry["status"] = "Failed"
-                errors = []
-                if client_result.returncode != 0:
-                    err = (
-                        client_result.stderr.strip().split("\n")[0]
-                        if client_result.stderr
-                        else (
-                            client_result.stdout.strip().split("\n")[0]
-                            if client_result.stdout
-                            else "No output"
-                        )
-                    )
-                    errors.append(f"Exit code {client_result.returncode}: {err[:80]}")
-                if missing:
-                    errors.append(f"Missing ({len(missing)}): {', '.join(missing[:5])}")
-                if mismatches:
-                    errors.append(f"Mismatch ({len(mismatches)}): {', '.join(mismatches[:3])}")
-                entry["error"] = " | ".join(errors)
-
-            results.append(entry)
-
         except Exception as e:
-            results.append(
+            results.append({
-                {"name": "Standard (no metadata)", "suite": profile_name, "status": "Error", "time": "N/A", "error": str(e)}
+                "name": name,
-            )
+                "suite": profile_name,
-        finally:
+                "status": "Error",
-            if server_process2:
+                "time": "N/A",
-                try:
+                "error": str(e),
-                    server_process2.wait(timeout=5)
+            })
-                except subprocess.TimeoutExpired:
-                    server_process2.kill()
-                    server_process2.wait()
 
-        # Rsync tests (over network via daemon, so tc netem applies)
+    return results
+
+
+def run_rsync_tests(profile_name, source_dir, dest_dir, client_prefix):
+    results = []
     rsync_port = find_free_port()
     rsyncd_conf = os.path.join(tempfile.gettempdir(), f"rsyncd-{rsync_port}.conf")
     with open(rsyncd_conf, "w") as f:
@@ -390,61 +289,39 @@ read only = yes
             rsync_args = case["args"]
             print(f"\n  --- {name} ---")
 
-                if os.path.exists(dest_dir):
+            cmd = (
-                    shutil.rmtree(dest_dir)
-
-                try:
-                    rsync_cmd = (
                 client_prefix
                 + ["rsync"]
                 + rsync_args
                 + [f"rsync://localhost:{rsync_port}/source/", f"{dest_dir}/"]
             )
-                    print(f"    Running: {' '.join(rsync_cmd)}")
+            print(f"    Running: {' '.join(cmd)}")
 
-                    start_time = time.monotonic()
+            try:
-                    rsync_result = subprocess.run(
+                result = run_single_test(
-                        rsync_cmd, capture_output=True, text=True, timeout=120
+                    cmd, name, source_dir, dest_dir,
+                    source_prefix=""
                 )
-                    end_time = time.monotonic()
+                result["suite"] = profile_name
-                    duration = end_time - start_time
+            except subprocess.TimeoutExpired:
-
+                result = {
-                    mismatches, missing = [], []
-                    if rsync_result.returncode == 0:
-                        mismatches, missing = verify_transfer(source_dir, dest_dir)
-
-                    entry = {
                     "name": name,
                     "suite": profile_name,
-                        "time": f"{duration:.4f}s" if rsync_result.returncode == 0 else "N/A",
+                    "status": "Timeout",
+                    "time": "N/A",
+                    "error": "Exceeded 120s",
+                }
+            except Exception as e:
+                result = {
+                    "name": name,
+                    "suite": profile_name,
+                    "status": "Error",
+                    "time": "N/A",
+                    "error": str(e),
                 }
-
-                    if rsync_result.returncode == 0 and not mismatches and not missing:
-                        entry["status"] = "Success"
-                        entry["error"] = ""
             else:
-                        entry["status"] = "Failed"
+                # If failed under systemd-run, retry without it to reveal raw error
-                        errors = []
+                if result["status"] != "Success" and client_prefix:
-                        if rsync_result.returncode != 0:
-                            errs = []
-                            for line in (rsync_result.stderr or "").split("\n"):
-                                line = line.strip()
-                                if not line:
-                                    continue
-                                if line.startswith("Running as unit:"):
-                                    continue
-                                errs.append(line)
-                            for line in (rsync_result.stdout or "").split("\n"):
-                                line = line.strip()
-                                if not line:
-                                    continue
-                                errs.append(line)
-                            if not errs:
-                                errs.append("No output")
-                            err = " | ".join(errs[-3:])
-                            errors.append(f"Exit code {rsync_result.returncode}: {err[:200]}")
-                            # Retry without systemd-run to reveal the actual error
-                            if client_prefix:
                     tmp_dest = tempfile.mkdtemp()
                     try:
                         plain = subprocess.run(
@@ -454,25 +331,11 @@ read only = yes
                         if plain.returncode != 0:
                             plain_errs = [l for l in (plain.stderr or "").split("\n") if l.strip()]
                             if plain_errs:
-                                            errors.append(f"raw: {plain_errs[-1][:150]}")
+                                result["error"] += f" | raw: {plain_errs[-1][:150]}"
                     finally:
                         shutil.rmtree(tmp_dest, ignore_errors=True)
-                        if missing:
-                            errors.append(f"Missing ({len(missing)}): {', '.join(missing[:5])}")
-                        if mismatches:
-                            errors.append(f"Mismatch ({len(mismatches)}): {', '.join(mismatches[:3])}")
-                        entry["error"] = " | ".join(errors)
 
-                    results.append(entry)
+            results.append(result)
-
-                except subprocess.TimeoutExpired:
-                    results.append(
-                        {"name": name, "suite": profile_name, "status": "Timeout", "time": "N/A", "error": "Exceeded 120s"}
-                    )
-                except Exception as e:
-                    results.append(
-                        {"name": name, "suite": profile_name, "status": "Error", "time": "N/A", "error": str(e)}
-                    )
 
     finally:
         if rsync_daemon:
@@ -486,8 +349,38 @@ read only = yes
         except Exception:
             pass
 
+    return results
+
+
+def run_profile(profile_name, source_dir, dest_dir):
+    is_limited = profile_name != "Unlimited"
+    params = NETWORK_PROFILES[profile_name]
+    print(f"\n{'=' * 60}")
+    print(f"Profile: {profile_name}")
+    print(f"{'=' * 60}")
+
+    if is_limited:
+        p = params
+        print(f"  Network: rate={p['rate']}, delay={p['delay']} ±{p['jitter']}, loss={p['loss']}")
+        print(f"  Disk I/O: Reads <= {READ_BPS_MAX}, Writes <= {WRITE_BPS_MAX}")
+        client_prefix = CLIENT_CMD_PREFIX
+    else:
+        print("  No limits applied")
+        client_prefix = []
+
+    try:
+        if is_limited:
+            netem_apply(profile_name)
+        else:
+            netem_reset()
+
+        results = []
+        results.extend(run_client_tests(profile_name, source_dir, dest_dir, client_prefix))
+        results.extend(run_rsync_tests(profile_name, source_dir, dest_dir, client_prefix))
+
     except subprocess.CalledProcessError as e:
         print(f"  Error running netem command: {' '.join(e.cmd)}")
+        results = []
     finally:
         if is_limited:
             try:
@@ -523,46 +416,7 @@ def format_throughput(bps):
     return f"{bps:.0f} B/s"
 
 
-def main():
+def print_results(all_results, total_bytes):
-    parser = argparse.ArgumentParser(description="FastSync integration test / benchmark")
-    parser.add_argument("--source-dir", default=DEFAULT_SOURCE_DIR,
-                        help="Source directory for test files (default: %(default)s)")
-    parser.add_argument("--dest-dir", default=DEFAULT_DEST_DIR,
-                        help="Destination directory for received files (default: %(default)s)")
-    parser.add_argument("--keep-data", action="store_true",
-                        help="Keep test_data directory after run")
-    parser.add_argument("--unlimited", action="store_true",
-                        help="Run Unlimited profile instead of LAN (no network limits)")
-    parser.add_argument("--wan", action="store_true",
-                        help="Run WAN profile instead of LAN (100mbit, 50ms, 1% loss)")
-    args = parser.parse_args()
-
-    os.system("cmake -B build -S . > /dev/null 2>&1")
-    ret = os.system("cd build && make -j$(nproc) 2>&1 | tail -3")
-    if ret != 0:
-        print("Build failed")
-        sys.exit(1)
-
-    total_bytes = generate_test_files(args.source_dir)
-    if os.path.exists(args.dest_dir):
-        shutil.rmtree(args.dest_dir)
-    os.makedirs(args.dest_dir, exist_ok=True)
-
-    profiles_to_run = []
-    if args.unlimited:
-        profiles_to_run.append("Unlimited")
-    elif args.wan:
-        profiles_to_run.append("WAN")
-    else:
-        profiles_to_run.append("LAN")
-
-    try:
-        all_results = []
-        for profile in profiles_to_run:
-            all_results.extend(
-                run_profile(profile, args.source_dir, args.dest_dir)
-            )
-
     print("\n" + "=" * 130)
     print(f"{'RESULTS':^130}")
     print("=" * 130)
@@ -632,6 +486,49 @@ def main():
     else:
         print(f"\n  ALL {len(all_results)} TESTS PASSED")
 
+
+def main():
+    parser = argparse.ArgumentParser(description="FastSync integration test / benchmark")
+    parser.add_argument("--source-dir", default=DEFAULT_SOURCE_DIR,
+                        help="Source directory for test files (default: %(default)s)")
+    parser.add_argument("--dest-dir", default=DEFAULT_DEST_DIR,
+                        help="Destination directory for received files (default: %(default)s)")
+    parser.add_argument("--keep-data", action="store_true",
+                        help="Keep test_data directory after run")
+    parser.add_argument("--unlimited", action="store_true",
+                        help="Run Unlimited profile instead of LAN (no network limits)")
+    parser.add_argument("--wan", action="store_true",
+                        help="Run WAN profile instead of LAN (100mbit, 50ms, 1% loss)")
+    args = parser.parse_args()
+
+    os.system("cmake -B build -S . > /dev/null 2>&1")
+    ret = os.system("cd build && make -j$(nproc) 2>&1 | tail -3")
+    if ret != 0:
+        print("Build failed")
+        sys.exit(1)
+
+    total_bytes = generate_test_files(args.source_dir)
+    if os.path.exists(args.dest_dir):
+        shutil.rmtree(args.dest_dir)
+    os.makedirs(args.dest_dir, exist_ok=True)
+
+    profiles_to_run = []
+    if args.unlimited:
+        profiles_to_run.append("Unlimited")
+    elif args.wan:
+        profiles_to_run.append("WAN")
+    else:
+        profiles_to_run.append("LAN")
+
+    try:
+        all_results = []
+        for profile in profiles_to_run:
+            all_results.extend(
+                run_profile(profile, args.source_dir, args.dest_dir)
+            )
+
+        print_results(all_results, total_bytes)
+
     finally:
         if not args.keep_data:
             shutil.rmtree(TEST_DIR, ignore_errors=True)