Skip to content

test(sdk): pass optional parameters in generated service tests - #2117

Merged
abnegate merged 2 commits into
mainfrom
test/sdk-service-optional-params
Oct 21, 2026
Merged

abnegate merged 2 commits into
mainfrom
test/sdk-service-optional-params

Conversation

@abnegate

@abnegate abnegate commented Oct 21, 2026 •

Copy link
Copy Markdown
Member

What

After #2114, each generated PHP, Python, Dart and Flutter method test checks the verb, the path and the parameter names it sends. But it calls the method with its required parameters only, so a method that drops an optional parameter, or drops a boolean one when it is false, still passes. Hansi raised this on console-python's storage.create_bucket in appwrite/sdk-for-console-python#22.

Each generated method test now passes every parameter the method accepts and checks the value sent for each one.

Example values. Every non-path boolean is false, so a dropped false fails the test. Everything else uses the values the templates already used: the spec example or the parameter name for strings, 1 for integers, 1.0 for numbers, [] for arrays, {} for objects, the first or example member for enums, a model built from its required properties, and an empty upload for files. Path booleans stay true so the path segment isn't empty. No spec has one today.

Per language:

  • PHP: required arguments are positional and optional ones use named arguments (fileSecurity: false). The Mockery::on closure still checks array_keys($params) with assertSame, which now lists every parameter. It then checks the values with assertSame($expected, array_intersect_key($params, $expected)). $expected covers every parameter except files, which the SDK turns into a Part, and models, which ::from() builds as a new instance on each call. Enums are cached instances, so assertSame works on them. Optional enums, models and InputFile are now imported.

  • Python: optional parameters are keyword arguments (file_security=False). The test compares the whole request with assertEqual:

    • JSON: the decoded body against a dict of every parameter and its value.
    • GET: parse_qs(url.query) against {'name': ['value']}, including the security query (project: ['']).
    • multipart: each form part's name and content against {'name': 'value'}. A file's content is the empty upload.

    Booleans arrive as 'false' in a query string or form part.

  • Dart/Flutter: every parameter is a named argument. The name now uses the service's own identifier (caseCamel | overrideIdentifier, e.g. xasync). expect(params.keys, unorderedEquals(...)) becomes expect(params, {...}), which checks the exact key set and the scalar values. Enums are compared as .value. Files, arrays and objects use isA<InputFile>(), isA<List>() and isA<Map>(). For webAuth methods, the test also checks each scalar parameter in url.queryParameters.

Unchanged from #2114: config-filled path parameters, deprecated-method skipping in PHP, chunked uploads, and webAuth/location handling. Each method still gets one test, so the test count doesn't change.

Evidence

New Generation test fails before and passes after. GenerationTest::testServiceTestsPassOptionalParameters has 13 cases. They cover the arguments and the asserted values for functions.createExecution (optional async: false) and general.getOptional (optional query parameters) in all four languages, plus the Python multipart case.

  • Original templates (origin/main): Tests: 22, Assertions: 22, Failures: 16. All 13 new cases fail. The other 3 are testServiceTestsAssertTheRequest fragments (python body, dart params, flutter params), which I updated to the new assertion form.
  • This PR: OK (22 tests, 22 assertions).

Mutation check. I mutated a method in each generated fixture SDK and ran only that method's generated test:

SDK mutation original template this PR
php Functions::createExecution sends async only when truthy (if ($async)) passes (missed) fails (caught)
php General::getOptional never sends name passes (missed) fails (caught)
python create_execution sends async only when truthy (if xasync:) passes (missed) fails (caught)
python get_optional never sends name passes (missed) fails (caught)
dart createExecution sends async only when true passes (missed) fails (caught)
dart getOptional never sends name passes (missed) fails (caught)
flutter getOptional never sends name passes (missed) fails (caught)

The client fixture has no optional boolean, so Flutter has no boolean row. Its template matches Dart's.

On the real 2.4.x console spec, I changed Python's storage.create_bucket to send fileSecurity only when it's truthy. test_create_bucket passes on main and fails with this PR. This is the bug class from appwrite/sdk-for-console-python#22.

Generated tests pass on the real Appwrite 2.4.x spec, generated locally:

  • PHP (server): OK (945 tests, 3525 assertions). On main it was 945 tests and 2765 assertions.
  • Python (console): 1070 passed, and black --check passes.
  • Dart (server): +1307: All tests passed!. dart analyze test reports no new issues.
  • Flutter (client): +200: All tests passed! (test/services)

On the fixture, PHP (217 tests, 617 assertions), Python (159 passed), Dart (+56) and Flutter (+55) all pass.

Known limits

  • Every test now passes every optional parameter, so the generated unit tests no longer check that an omitted optional parameter is left out of the request. The tests on main checked this, because they called each method without its optional parameters.
  • As in test(sdk): assert the request in generated service tests #2114, Python's query-string and multipart encoding drop the empty []/{} arguments, so GET and multipart expectations leave out array and object parameters. JSON bodies include them.

🤖 Generated with Claude Code

Generated PHP, Python, Dart and Flutter service tests called each method
with its required parameters only, so a method that dropped an optional
parameter, or dropped a boolean one when it was false, still passed. Each
test now passes every parameter the method accepts, with booleans as
false, and asserts the value sent for each one; files, models, arrays and
objects are asserted by key where their value is not comparable.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@hansi-codes

hansi-codes Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

🟢 Tier S · Ready to merge

The PHP, Python, Dart, and Flutter service-test templates now pass optional arguments and assert the request values they send. Generation tests add coverage for optional booleans, query/body parameters, and multipart values across the generated SDKs.

Verdict New comments Fixed Still open
✅ Approved 0 0 0
📂 Walkthrough · 5
File Change
templates/dart/test/services/service_test.dart.twig Passes all method parameters and asserts serialized values in request maps and webAuth URLs.
templates/flutter/test/services/service_test.dart.twig Applies the Dart service-test argument and request-value checks to Flutter.
templates/php/tests/Services/ServiceTest.php.twig Passes optional parameters by name and asserts parameter values where comparable.
templates/python/test/services/test_service.py.twig Passes optional parameters and compares JSON, query-string, and multipart request values.
tests/generation/GenerationTest.php Adds generation assertions for optional parameters and updated request-value expectations.
🔇 Filtered out · 1

Findings Hansi considered but did not post.

Finding Why
Model parameters are asserted as objects, not serialized maps Verifier: Already handled by sent_value's object branch (lines 90–91). SDK::getSchemaType (src/SDK/SDK.php:1623–1635) classifies ReferenceSchema and non-string-enum CompositeSchema as 'object', so a model-typed parameter produces isA(), not argument_value's model constructor. Model arrays produce isA(). These match the service's .toMap() / mapped-list serialization. The Flutter template has the same handling.

Merged main (cdc0e83); no changes to this PR's code since 9917e21 · Details · Comment @hansi-codes review to re-run, or mention @hansi-codes with a question.

@hansi-codes hansi-codes Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Tier S · Looks good to merge. Summary

@abnegate
abnegate merged commit eca2a90 into main Oct 21, 2026
60 checks passed
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