Repository navigation
fix: fal unit tests - #1216
fix: fal unit tests#1216
Conversation
There was a problem hiding this comment.
Looks good — straightforward bug fixes with matching tests.
What was reviewed: the DistributedWorker.submit/shutdown fix (closes unscheduled coroutines on failure, early-returns on repeated shutdown) and its new unit tests; the pydantic v2 fix in toolkit/types.py (dropped deprecated update_not_none for plain dict.update, added missing @ classmethod on __get_pydantic_json_schema__) and its new schema tests; and the test-isolation infrastructure (conftest changes preventing local FAL_CONFIG_PATH/FAL_PROFILE from leaking into unit tests, plus a cli_working_directory fixture replacing live filesystem lookups in deploy tests).
Extended reasoning...
Diff touches fal/distributed/worker.py (coroutine submission/shutdown lifecycle), fal/toolkit/types.py (pydantic v2 schema customization), and six test files; no auth/crypto/permission surface is touched. The two source fixes are small, mechanical, and verified correct by reading the full method bodies; test changes are internally consistent isolation/fixture work with no risky behavior. Size and clarity support approval without requiring a human look.
Note
Low Risk
Changes are mostly test harness and small resource-cleanup fixes; toolkit schema tweaks affect OpenAPI/JSON schema output but not runtime validation logic.
Overview
Hardens distributed workers and Pydantic toolkit types, while isolating and stabilizing the fal unit test suite.
In
worker.py,submit()now closes the coroutine if scheduling fails (e.g. loop not running), avoiding leaked coroutines;shutdown()no-ops when already stopped so repeated shutdown is safe. Infal.toolkit.types, Pydantic v2 JSON schema hooks are fixed (@classmethodon__get_pydantic_json_schema__) and schemaformatis set viafield_schema.updateinstead of removedupdate_not_none.Test changes decouple unit runs from the developer’s local fal config: root
conftestpatchesFAL_CONFIG_PATHwhen collecting onlytests/unit, addstests/unit/conftest.pyautouse isolation, and moves GRPC/auth debug output topytest_report_headerwith lazy imports. Deploy CLI tests use acli_working_directoryfixture instead offind_project_root. WMA tests get a shared_restore_aioice_closeautouse fixture (removed from individual tests). New coverage covers worker submit/shutdown, toolkit JSON schema formats, and subprocess verification that unit collection ignores invalid local config.Reviewed by Cursor Bugbot for commit 5a2901a. Bugbot is set up for automated code reviews on this repo. Configure here.