Fable 5 test: reorganize test files and consolidate generator modules#257
Merged
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
phalt
changed the title
split_ver() parsed platform.python_version() with int(), which raises ValueError on pre-release interpreters (e.g. 3.14.0rc2 or the 3.15-dev build in the CI matrix) and made the package unimportable there. sys.version_info already provides the integer components we need. https://claude.ai/code/session_012AGyWgJj3Mp5aNFJ5qAB4s
BaseClientsGenerator had exactly one subclass, and the only method that justified the split - its generate_function - rendered templates (get_method.jinja2, post_method.jinja2) that do not exist anywhere in the repository, so it could never run. The base/subclass split also forced the FrameworkHTTPPlaceholder shim, whose only job was to carry a dict between methods of what is logically one class. Everything now lives in a single ClientsGenerator, with the status-code bundle as a plain attribute. Also removed along the way, all verified unused by code or templates: - ParametersResponse.get_path_args_as_string / get_required_path_args_as_string / has_query_args - shared/utils create_query_args and create_query_args_with_mapping (only callers were the dead generate_function) - the new_unions template variable (no template references it) Regenerating the test clients produces byte-identical output. https://claude.ai/code/session_012AGyWgJj3Mp5aNFJ5qAB4s
The package nested a second generators/ directory inside both the api and shared packages (clientele/generators/api/generators/, clientele/generators/shared/generators/), and api/generators/schemas.py was a four-line re-export of the shared SchemasGenerator. The extra level added indirection without grouping anything - each inner directory held a single real module. Now: - clientele/generators/api/clients.py (was api/generators/clients.py) - clientele/generators/shared/schemas.py (was shared/generators/schemas.py) - the re-export module is gone; callers import the shared SchemasGenerator directly https://claude.ai/code/session_012AGyWgJj3Mp5aNFJ5qAB4s
Two modules each held one function with a single caller: - clientele/utils.py only contained get_client_project_directory_path, which is used exclusively by the generators at code-generation time. It now lives in clientele/generators/shared/utils.py with the rest of the generator helpers, so the top-level package no longer has a utils.py whose contents are unrelated to the runtime library. - clientele/generators/schema_utils.py only contained build_union_type_string, called once from the shared SchemasGenerator. It is now defined in clientele/generators/shared/schemas.py next to its caller. https://claude.ai/code/session_012AGyWgJj3Mp5aNFJ5qAB4s
A module named requests inside an HTTP client library reads as if it wraps the requests package (which the project also supports as a backend via http/requests_backend.py). It actually defines RequestContext, PreparedCall and the validation that builds them, and it is only imported internally, so the rename cannot break users. https://claude.ai/code/session_012AGyWgJj3Mp5aNFJ5qAB4s
test_cli.py, test_cli_coverage.py and test_cli_full_coverage.py all tested clientele/cli.py, each redefining the same runner and OpenAPI spec fixtures, and several tests covered the same branch (the url-or-file assertion was tested three times, plain 3.0 file loading twice). The unique cases - Swagger 2.0 auto-conversion, the pre-3.0 version rejection in _prepare_spec, and OpenAPI 3.1 normalization - now live in tests/test_cli.py alongside the existing tests, sharing one set of fixtures. https://claude.ai/code/session_012AGyWgJj3Mp5aNFJ5qAB4s
tests/generators/shared/test_utils.py and test_utils_coverage.py both tested clientele/generators/shared/utils.py, re-testing snake_case_prop and get_type with overlapping inputs. The unique inputs from the coverage file are now parametrize cases or named tests in test_utils.py. Also absorbed here, since their subject moved into shared/utils.py in an earlier commit: the get_client_project_directory_path tests from tests/test_utils.py, and the get_param_from_ref/get_schema_from_ref edge cases from tests/test_additional_coverage.py. The rest of test_additional_coverage.py duplicated existing tests (the allOf cases are covered by test_get_type_with_allof and test_schemas_generator_handles_allof_with_object_properties), so the file is gone. Tests of private helpers with vacuous assertions (_split_upper) were dropped. https://claude.ai/code/session_012AGyWgJj3Mp5aNFJ5qAB4s
tests/test_generator_coverage.py mixed tests for two unrelated modules and tests/test_edge_case_coverage.py was a leftover catch-all. Their tests now live with the other tests for the module they exercise: - the normalize_openapi_31_* tests moved to tests/generators/test_cicerone_compat.py, which already tested that module - the SchemasGenerator no-components/no-schemas tests moved to tests/generators/test_schemas_coverage.py (dropping the duplicate copy from test_edge_case_coverage.py) - the APIGenerator regeneration and server-URL tests moved to tests/generators/framework/test_framework_generator_coverage.py, rewritten to build their specs with parse_spec_from_dict instead of tempfile round-trips https://claude.ai/code/session_012AGyWgJj3Mp5aNFJ5qAB4s
Tests for the generators were spread between the tests/ root (seven files all exercising clientele/generators/api), a tests/generators/ root with misplaced files, and a tests/generators/framework/ directory whose name matched nothing in the package. The tree now follows the source tree, so the tests for a module are where you expect them: - tests/generators/api/ APIGenerator and ClientsGenerator tests, including the feature/regression suites that lived at the tests/ root (complex schemas, $ref handling, nullable fields, OpenAPI 3.1 null types, issue 248, real-world fixture specs) and the former framework/ tests, renamed test_framework_*/test_standard_* -> test_api_generator_* - tests/generators/basic/ BasicGenerator tests (split out of test_base_and_basic_coverage.py; its ClientsGenerator half went to tests/generators/api/test_clients.py) - tests/generators/shared/ test_schemas.py (was tests/generators/test_schemas_coverage.py) next to test_utils.py - tests/test_testing.py renamed from test_testing_utilities.py to mirror clientele/testing.py __init__.py files added to the new directories so the two test_generator.py modules get unique import names. https://claude.ai/code/session_012AGyWgJj3Mp5aNFJ5qAB4s
Follow-ups from reviewing the refactor: - ClientsGenerator kept two seams that only existed so the deleted base class could be specialized: writer stored as an instance attribute aliasing the module, and the constant method template map rebuilt per instance. Both suggested override points that no longer exist; the template map is now a module-level constant and writer is used directly. - SchemasGenerator.generated_response_class_names was a mutable class-level list that nothing reads or writes - removed. - The api generator still described itself as 'the framework generator' in docstrings and comments, terminology this branch otherwise removed. Docstrings, the package comment, and the matching test name now say api. https://claude.ai/code/session_012AGyWgJj3Mp5aNFJ5qAB4s
- Make every tests/ directory a package: tests/generators/* gained __init__.py during the reorganization while tests/api, tests/cache, tests/http, tests/graphql and tests/mypy had none, leaving two import conventions and a collision trap for same-named test files in non-package directories. - tests/generators/api/test_clients.py: replace the identical 10-line generator construction repeated in all three tests with one helper, and use tmp_path instead of tempfile boilerplate. - tests/test_cli.py: use the existing get_spec_path() helper instead of re-deriving the example_openapi_specs path from __file__. - tests/generators/basic/test_generator.py: drop docstring references to physical line numbers in the module under test. https://claude.ai/code/session_012AGyWgJj3Mp5aNFJ5qAB4s
phalt
force-pushed
the
claude/codebase-refactor-tests-dc24y3
branch
from
05029f3 to
a8afd34
Compare
phalt
commented
Jun 10, 2026
phalt
left a comment
phalt
left a comment
Owner
Author
There was a problem hiding this comment.
Genuinely useful changes - mostly code reorganisation that I haven't been arsed to do myself, or previous models have failed and got caught in a deep logic loop trying to figure out.
| @@ -1,10 +1,5 @@ | |||
| import platform | |||
| import sys | |||
Owner
Author
There was a problem hiding this comment.
This is a legitimately better change.
| @@ -1,35 +1,62 @@ | |||
| """Generates the client.py file for the api generator.""" | |||
Owner
Author
There was a problem hiding this comment.
Left over from the recent refactor - this sort of consolidation makes total sense.
| @@ -11,7 +11,7 @@ | |||
|
|
|||
Owner
Author
There was a problem hiding this comment.
A genuinely sensible change.
| @@ -101,17 +101,37 @@ def test_load_openapi_spec_requires_url_or_file(): | |||
| cli._load_openapi_spec() | |||
Owner
Author
There was a problem hiding this comment.
Previous iterations of models to generate tests for test coverage naively did so with a new file - this shows how sensible Fable can be at realising it can all be consolidated now (maybe previous models could have done it too - who knows).
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
3 participants
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.
Change summary
This PR reorganizes the test suite structure and consolidates generator modules to improve code organization and maintainability.
Key Changes
Test Reorganization
Consolidated coverage tests: Merged multiple
*_coverage.pytest files into their corresponding main test files:test_generator_coverage.py→tests/generators/test_cicerone_compat.pytest_utils_coverage.py→tests/generators/shared/test_utils.pytest_cli_coverage.pyandtest_cli_full_coverage.py→tests/test_cli.pytest_additional_coverage.py→ Distributed to appropriate test modulestest_edge_case_coverage.py→tests/test_cli.pyReorganized generator tests:
tests/generators/framework/test_framework_generator_*.py→tests/generators/api/test_generator.pytests/generators/basic/test_generator.pyfor BasicGenerator testsRenamed test files for clarity:
tests/api/test_requests.py→tests/api/test_request_context.pytests/generators/api/test_clients.py(refactored)tests/generators/shared/test_schemas.py(refactored)Module Consolidation
schema_utils.pyfunctions: Consolidatedbuild_union_type_string()fromclientele/generators/schema_utils.pyintoclientele/generators/shared/schemas.pyget_client_project_directory_path()fromclientele/utils.pytoclientele/generators/shared/utils.pyclientele/generators/api/generators/schemas.py(re-export module)clientele/generators/api/generators/clients.py(unused framework-specific code)clientele/utils.py(functions moved to appropriate locations)clientele/generators/schema_utils.py(functions consolidated)Import Updates
from clientele.api import requests→from clientele.api import request_contextfrom clientele.generators.shared.generators.schemas import SchemasGenerator→from clientele.generators.shared.schemas import SchemasGeneratorfrom clientele.generators.api.generators import clients→from clientele.generators.api import clientsCode Quality Improvements
utils.py: Addedget_client_project_directory_path()function with improved path handlingsettings.py: Replaced customsplit_ver()function with direct use ofsys.version_infoRelated issue(s)
N/A
Pull request tasks
The following have been completed for this task:
make test(all tests pass)make formatmake tymake generate-test-clientsOther information
This refactoring improves code organization by:
https://claude.ai/code/session_012AGyWgJj3Mp5aNFJ5qAB4s