mng/modal-testing-what#897
Open
joshalbrecht wants to merge 10 commits intojosh/modal_plus_testingfrom
Open
Conversation
…rs in testing_provider_test.py Adds new unit tests to increase coverage of instance.py using TestingModalInterface (no real Modal credentials needed): - Tag operations (get_host_tags, set_host_tags, add_tags_to_host, remove_tags_from_host, rename_host) with both offline and sandbox paths - delete_snapshot including not-found and host-not-found error paths - get_host edge cases: cache hit, name-based search through host records - discover_hosts with mixed states (stopped, failed, destroyed) - discover_hosts_and_agents with mixed states and agent data - ModalProviderApp get_captured_output and close callbacks - ModalProviderInstance get_captured_output and close delegation - Volume proxy type mapping (_proxy_file_entry_type_to_volume_file_type) - Parsing helpers (_parse_optional_int, _parse_optional_float) with valid, empty, and invalid input - _parse_build_args with various argument formats Coverage increased from 71% to 73.23%. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Create a local testing implementation of ModalInterface in modal_proxy that fakes Modal behavior using local resources: - Volumes backed by real directories on disk - Sandboxes that run commands via subprocess - Lightweight no-op images, apps, and environments - In-memory secrets and function tracking Add TESTING mode to ModalProviderConfig and update the backend to support constructing a ModalProviderInstance with the testing interface. Write 160+ new tests for mng_modal that exercise business logic without requiring Modal credentials or SSH connections, covering: - Host record CRUD and caching - Agent data persistence and listing - Volume operations (state volumes and host volumes) - Sandbox caching and discovery - Host lifecycle (stop, destroy, delete) - Tag operations (get, set, add, remove, rename) - Snapshot management (list, delete) - Build args parsing and image building - Backend app registry management - Environment creation and retry logic Coverage improved from 45% to 76% for mng_modal. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Test create_snapshot and stop_host with pre-populated host cache to exercise the snapshot creation and stop lifecycle paths without SSH. Also add tests for discover_hosts edge cases with running sandboxes. Coverage for mng_modal is now 77.49%. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Test get_host_resources with missing record, _build_modal_image with docker_build_args and context_dir, get_connector error paths, and various listing/discovery edge cases. Fix _make_host_record to support config=None for testing failed hosts. Coverage for mng_modal is now 77.60% (up from 45%). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…tions - Move fixtures to conftest.py per project convention - Extract _make_snapshot() and _setup_host_with_sandbox() helpers to reduce duplication across tag/snapshot tests - Move make_testing_modal_interface and make_testing_provider to a testing.py module for shared use between conftest and tests - Fix vacuous assertions (assert len(x) >= 0) with meaningful checks - Add logging to deploy() error handler in TestingModalInterface - Remove test that only tested the test helper function Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
move helpers to testing.py, consolidate duplicate tests - Delete testing_test.py per CLAUDE.md (no tests for test utilities) - Remove isinstance(TestingModalInterface) from production code; use is_testing parameter threaded from ModalMode.TESTING enum instead - Move all test helper functions (make_host_record, make_snapshot, make_sandbox_with_tags, setup_host_with_sandbox) to testing.py per project convention for non-fixture test utilities - Remove 5 duplicate/overlapping tests (discover_hosts variants, stop_host duplicate) - Parameterize parse_optional_int/float tests with @pytest.mark.parametrize Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Use ConcurrencyGroup for process lifecycle management in the testing sandbox instead of raw subprocess.Popen/subprocess.run. This provides proper process tracking and automatic cleanup via the existing concurrency infrastructure. - TestingModalInterface now takes a ConcurrencyGroup parameter - Each sandbox creates a child ConcurrencyGroup for its processes - Background commands use cg.run_process_in_background() - Blocking commands use cg.run_process_to_completion() - Terminate uses RunningProcess.terminate() instead of os.kill() - Removed all direct subprocess imports from testing.py - Reverted subprocess ratchet count back to original value (2) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a "testing" implementation to the modal_proxy library
Stacked on #887