Skip to content

Testability refactor#388

Closed
lneely wants to merge 11 commits intomainfrom
testability-refactor
Closed

Testability refactor#388
lneely wants to merge 11 commits intomainfrom
testability-refactor

Conversation

@lneely
Copy link
Owner

@lneely lneely commented Mar 11, 2026

No description provided.

Levi Neely and others added 11 commits March 10, 2026 19:19
Adds tests/unit-tests/test_ptask_free.c to verify all code paths of the
psync_task_free fix from #377: single-owner free, last-ref destroy,
non-last-ref decrement, READY task signaling, and lock-before-refcnt
ordering. All 5 tests pass. Also adds compiled binary to .gitignore.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Extract psync_task_free + psync_task_destroy (and their static helpers
psync_task_dec_refcnt, psync_task_entry) from ptask.c into a new
separately-compilable unit pclsync/ptask_free.c. Add
pclsync/ptask_free_internal.h to expose the internal struct layout
(struct psync_task_manager_t_ / struct psync_task_t_) for test use
without pulling in ptask.c's heavyweight transitive dependencies.

Rewrite tests/unit-tests/test_ptask_free.c to:
- Include ptask_free_internal.h instead of duplicating structs inline
- Call the real psync_task_free() rather than a local replica
- Intercept pthread_mutex_lock/unlock and pmem_free via --wrap linker
  flags to observe lock discipline and detect destroy invocations

Update the Makefile test_ptask_free target to link pclsync/ptask_free.c
and pass the required --wrap flags. Production build unchanged: ptask_free.o
is picked up automatically by the existing wildcard COBJ rule.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Task #3 — Unit tests for ptree and pintervaltree
  tests/unit-tests/test_ptree.c: 8 tests covering single-node insert,
  in-order traversal after arbitrary and reverse inserts, BST lookup,
  leaf/root/all-node deletion, and ptree_for_each visitation.
  tests/unit-tests/test_pintervaltree.c: 18 tests covering single add,
  non-overlapping, overlapping/adjacent/contained/spanning merges,
  chain merge, remove middle split, remove exact/left/right/spanning,
  cut_end, first_interval_containing_or_after, and free(NULL).

Task #4 — Extract pfstasks tree layer
  pclsync/pfstasks_tree.h + pclsync/pfstasks_tree.c: pure tree layer
  (zero psql calls) extracted from pfstasks.c — pfs_task_search_tree,
  pfs_task_walk_tree (static helpers), pfs_task_insert_into_tree,
  pfs_task_find_mkdir/rmdir/creat/unlink,
  pfs_task_find_mkdir_by_folderid, pfs_task_find_creat_by_fileid.
  pclsync/pfstasks.c: #includes pfstasks_tree.h; all moved functions
  removed; all callers unchanged.
  tests/unit-tests/test_pfstasks_tree.c: 13 tests using direct tree
  construction (no DB) to verify find-by-name, taskid discrimination,
  find-by-numeric-id, and empty-folder edge cases.

Task #5 — psql in-memory harness + pfstasks DB tests
  tests/helpers/psql_test_helpers.h + .c: lightweight harness that
  opens :memory: via sqlite3_open, enables PRAGMA foreign_keys=ON, and
  applies the full PSYNC_DATABASE_STRUCTURE schema. Exposes
  psql_test_db(), psql_test_exec(), psql_test_insert_fstask(),
  psql_test_count_fstask/fstaskdepend(). No dependency on psql.c.
  tests/unit-tests/test_pfstasks_db.c: 10 tests verifying schema
  creation, fstask insertion/query, fstaskdepend insertion, CASCADE
  DELETE propagation, FK enforcement, rmdir-blocking SQL pattern,
  creat-after-unlink sequencing, and open/close idempotence.

All 11 new tests pass; production build clean.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1. Check psql_test_exec() return values in test_cascade_delete() and
   test_creat_after_unlink() consistently with test_fstaskdepend_insert().
2. Remove dead dep_cnt variable and (void)dep_cnt suppressor from
   test_creat_after_unlink().
3. Change SQLITE_STATIC → SQLITE_TRANSIENT for text1 binding in
   psql_test_insert_fstask() to avoid dangling-pointer footgun on
   future reuse.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…l leak

pclsync/ppath.c: Move buff[4096] to function scope in ppath_home() so
the pointer stored in dir via result->pw_dir remains live through the
putil_strdup(dir) call. Previously buff went out of scope at the if-block
close, causing a stack-use-after-scope ASAN report on every call that fell
through the getpwuid_r path.

tests/unit-tests/test_ptools_errptr.c: run_unfixed() intentionally leaks
errPtr to demonstrate the pre-fix bug. Wrap the allocation with
LSAN_DISABLE() / LSAN_ENABLE() so LSAN does not abort the process at exit
before stdio flushes, which was causing a non-zero exit code. The guard
uses nested #ifdef/__has_feature to remain compatible with both GCC
(__SANITIZE_ADDRESS__) and Clang (__has_feature(address_sanitizer))
without triggering "missing binary operator" errors on GCC.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Task #11 — plocks.c stress test (test_plocks.c)
  7 tests: basic rdlock/wrlock round-trip, recursive TLS counting (same
  thread acquires rdlock N times; unlock only releases on final decrement),
  upgrade under contention (N readers + towrlock; barrier-synchronized),
  writer starvation prevention (sustained reader load; writer acquires
  within 500ms), N-reader + M-writer counter-integrity stress test (ASAN),
  and try-variant contention (trywrlock fails when another thread holds
  rdlock).  TSAN note documented: custom lock internals require ASAN-only
  when ThreadSanitizer annotations are absent.

Task #12 — pfsupload send-function tests (pfsupload_send.c/h + test_pfsupload.c)
  Extract psync_send_task_mkdir and psync_send_task_rmdir from pfsupload.c
  into pclsync/pfsupload_send.c as non-static pfsupload_send_mkdir/rmdir.
  Expose fsupload_task_t struct via pclsync/pfsupload_send.h.  Add
  __attribute__((weak)) get_urls() as an injectable URL seam for large-
  upload paths.  pfsupload.c updated to include pfsupload_send.h and use
  the renamed functions in its dispatch table; pfsupload_send.o is
  automatically picked up by the production wildcard build.

  5 tests: mkdir (non-encrypted) command + folderid param, mkdir
  (encrypted) key param present, rmdir command + sfolderid, API error path
  (papi_send failure → -1), get_urls() weak override.  Uses
  --wrap=papi_send to intercept API calls and socketpair() to provide a
  valid psock_t without real network I/O.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1. test_plocks.c: add comment to test_upgrade_under_contention clarifying
   it verifies towrlock completion and holding_wrlock; notes that concurrent
   exclusivity is covered by test_stress().

2. test_pfsupload.c / make_fake_api: replace static-local psock_t with
   caller-supplied stack allocation (out parameter) to eliminate the
   multiple-calls-per-test footgun.

3. test_pfsupload.c / find_str_param: replace ternary
   `paramnamelen == strlen ? paramname : ""` with explicit length check +
   strncmp, matching the cleaner pattern used in find_num_param.

4. Makefile: add comment next to -Wl,--wrap=papi_send noting it redirects
   papi_send to __wrap_papi_send and is GNU ld only (not macOS Apple ld).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Task #19 — ppagecache.c decomposition (ppagecache_helpers.c/h)
  ppagecache_compute_page_priority(usecnt): pure function returning the
  LRU eviction tier (0–4) that matches the five pagecache_entry_cmp_*
  sort comparators in ppagecache.c (thresholds 2/4/8/16).
  ppagecache_verify_crc(data, size, stored_crc): wraps pcrc32c_compute
  and compares; returns 0 on match, -1 on mismatch.
  ppagecache_get_download_urls(fileid, hash, nout): __attribute__((weak))
  URL-injection seam; default returns NULL (falls through to real API).
  ppagecache.c updated to include ppagecache_helpers.h.

  test_ppagecache.c: 10 tests covering tier boundary conditions (0/1/2/3/4
  including UINT32_MAX), CRC match, single-bit flip, wrong stored CRC,
  zero-length buffer, and weak URL override.

Task #20 — pfs.c helper extraction (pfs_helpers.c/h)
  pfs_row_to_folder_stat(row, stbuf): converts psql folder row → struct
  stat; uses pfs_task_get_folder_tasks_rdlocked for in-memory mtime.
  pfs_row_to_file_stat(row, stbuf, flags): converts psql file row → stat;
  encrypted path calls pfs_crpt_plain_size.
  pfs_mkdir_to_folder_stat(mk, stbuf): converts in-memory mkdir task →
  stat (no SQL).
  pfs_apply_task_overlay(stbuf, folder, name, flags): applies pending
  mkdir/rmdir/unlink/creat-new overlays from the in-memory task queue;
  returns 1/2/-1/0.
  pfs_stat_uid/gid: exported globals; pfs.c syncs them from myuid/mygid.
  pfs.c updated to #include pfs_helpers.h and sync the uid/gid globals.
  pfsfolder.c: pfs_fldr_resolve_path decorated __attribute__((weak)) so
  tests can inject fake path resolution without a FUSE mount or psql.

  test_pfs_helpers.c: 7 tests covering folder/file stat field correctness,
  overlay NULL/mkdir/rmdir/no-match cases, and weak path override; uses
  --wrap for pfs_task_get_folder_tasks_rdlocked, pfs_crpt_plain_size, and
  ptimer_time to stay SQL/crypto/timer-free.

Production build clean; make check exits 0.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Keep test_ppagecache and test_pfs_helpers entries from testability-refactor
alongside test_pfsupload already merged into main.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- pcryptofolder.c: psync_cloud_crypto_send_mkdir, psync_pcloud_crypto_reencode_key, psync_pcloud_crypto_encode_key
- psynclib.c: psync_set_ssl_debug_callback, psync_reset_apiserver, create_bup_mach_folder, psync_async_delete_sync
- pfscrypto.c: pfs_crpt_write_mod_nu
- ppagecache.c: clean_cache_del
- publiclinks.c: process_bres, do_delete_all_links
- pfsupload.c: upload_modify
- debug/pnetlibs_debug.c: pident
- pnetlibs.c: psync_socket_close_download
- pfstasks.c: pfs_task_stop_and_delete_file
- pssl.c: rng_get
- pupload.c: handle_api_errors

Co-authored-by: Levi Neely <lkn@darkstar.example.net>
@lneely lneely closed this Mar 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant