Skip to content

Unit Tests - Python#738

Draft
mvarendorff wants to merge 5 commits intoappwrite:masterfrom
mvarendorff:feat-680-add-unit-tests-python
Draft

Unit Tests - Python#738
mvarendorff wants to merge 5 commits intoappwrite:masterfrom
mvarendorff:feat-680-add-unit-tests-python

Conversation

@mvarendorff
Copy link
Copy Markdown
Contributor

@mvarendorff mvarendorff commented Nov 4, 2023

What does this PR do?

This PR adds generated unit tests to the Python SDK.

Test Plan

Generate the SDK, weave hands to get the dependencies installed (my IDE did it for me, I have no clue about Python, I am sorry!), then run python3 -m unittest.

Related PRs and Issues

#680

Have you read the Contributing Guidelines on issues?

Yup


Discord username for swag as requested by Tessa: yestheory

@mvarendorff mvarendorff marked this pull request as draft November 19, 2023 14:33
@mvarendorff mvarendorff force-pushed the feat-680-add-unit-tests-python branch from 0528146 to ebc8868 Compare November 19, 2023 15:40
@mvarendorff mvarendorff marked this pull request as ready for review November 19, 2023 15:40
@abnegate
Copy link
Copy Markdown
Member

Are you make these test run automatically with the existing tests?

@lohanidamodar
Copy link
Copy Markdown
Member

Are you make these test run automatically with the existing tests?

@abnegate I don't think we can do that yet. But we want to use these test to run in the appwrite/sdk-for-python with github acitons.

@lohanidamodar
Copy link
Copy Markdown
Member

@abnegate let's get this merged in a different branch and sync

@ChiragAgg5k
Copy link
Copy Markdown
Member

Quick triage check: is this PR still active? It has been inactive for a long time and there is newer testing work in the repo now. If you still plan to continue, please share status/rebase plan; otherwise we can close it for now.

@mvarendorff mvarendorff force-pushed the feat-680-add-unit-tests-python branch from ebc8868 to 864a059 Compare April 1, 2026 06:37
@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Apr 1, 2026

Greptile Summary

This PR adds generated unit tests to the Python SDK, covering Query, Role, Permission, ID, Operator, and per-service HTTP-mock tests. It also introduces the responseModelExample Twig filter (backed by a new PHP method in Python.php) to produce fixture data for those service tests, and wires python -m unittest into the CI build validation workflow.

The individual test templates are well-written and cover a good breadth of the SDK surface area. However, there are two bugs in the getResponseModelExample PHP method that need to be fixed before the service tests can work correctly for all spec models:

  • Debug echo left in + dead 'object' match arm: An echo $property / return '' block (lines 764–767) fires for every object-typed property, short-circuiting the recursive getResponseModelExample call in the match arm that was meant to handle it. This corrupts generated output and leaves the recursive branch unreachable.
  • Missing falseFalse / nullNone replacements: The JSON→Python literal substitution on line 780 only handles trueTrue; false and null are not valid Python identifiers, so any spec property whose resolved example value is false or null will produce a syntax error in the generated test file.

Confidence Score: 4/5

Two P1 bugs in the PHP code-generation logic must be fixed before service tests will work correctly for all API models.

The test templates themselves are sound and the CI integration is straightforward, but the getResponseModelExample helper has a leftover debug statement that breaks object-property handling and an incomplete JSON→Python literal conversion that would produce invalid Python for false/null example values. Both issues are in the same small function and are straightforward to fix.

src/SDK/Language/Python.php — specifically the getResponseModelExample method (lines 764–780).

Important Files Changed

Filename Overview
src/SDK/Language/Python.php Adds new Twig filter responseModelExample and supporting getResponseModelExample PHP method, plus file-mapping entries for the new test templates. Contains two bugs: a debug echo statement that both corrupts output and makes the recursive 'object' branch dead code, and missing JSON→Python literal replacements for false/null.
templates/python/test/services/test_service.py.twig Generates per-service test files that mock HTTP responses and assert the SDK model round-trips correctly via to_dict(); logic is sound but depends on the object-type fix in Python.php to produce valid test data.
templates/python/requirements.txt.twig Adds requests_mock==1.11.0 as a test dependency; exact pin is inconsistent with the range-based versioning used for other dependencies.
templates/python/test/test_query.py.twig Comprehensive query builder tests covering all filter, cursor, ordering, and select methods; uses {% verbatim %} correctly to prevent Twig from interpreting Python f-string braces.
templates/python/test/test_operator.py.twig Comprehensive operator tests covering arithmetic, array, string, and date operations; all expected JSON serialisations look correct.
.github/workflows/sdk-build-validation.yml Adds python -m unittest to the Python CI step so the generated tests are run during SDK build validation.

Reviews (1): Last reviewed commit: "WIP fix: tests" | Re-trigger Greptile

Comment on lines +764 to +770
if ($property['type'] === 'object') {
echo $property;
return '';
}

$result[$property['name']] = match ($property['type']) {
'object' => $this->getResponseModelExample($property['sub_schema'], $spec),
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Debug echo statement and dead 'object' match arm

Lines 764–767 contain a debug echo $property (calling echo on a PHP array triggers a "Array to string conversion" notice and literally outputs the string "Array" to stdout) followed by return ''. Because this early-return guard fires for every property whose type is 'object', the 'object' arm in the match expression on line 770 can never execute — it is dead code.

The intent of the function is clearly to recurse into nested response models for object-typed properties, but the debug block prevents that. The correct fix is to remove the dead guard block entirely so the match handles it:

            $result[$property['name']] = match ($property['type']) {
                'object' => $this->getResponseModelExample($property['sub_schema'], $spec),
                'array' => [],
                'string' => $property['example'] ?? '',
                'boolean' => true,
                default => $property['example'],
            };


$json = json_encode($result, JSON_PRETTY_PRINT);

return str_replace('true', "True", $json);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Missing falseFalse and nullNone replacements

Only true is replaced with its Python equivalent True. JSON also uses false and null, which are invalid Python literals — Python expects False and None respectively. If any property's example value in the spec resolves to PHP false or PHP null (serialised via the default branch of the match), the generated .py test file will contain a syntax error.

$json = json_encode($result, JSON_PRETTY_PRINT);
return str_replace(
    ['true', 'false', 'null'],
    ['True', 'False', 'None'],
    $json
);

@@ -1,2 +1,3 @@
requests>=2.31,<3
requests_mock==1.11.0
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Exact version pin inconsistent with other dependencies

requests_mock is pinned to exactly 1.11.0 with ==, while requests and pydantic both use flexible ranges (e.g. >=2.31,<3). An exact pin means the package won't automatically receive patch-level bug fixes and may cause resolution conflicts in environments that already have a different requests_mock version installed. Consider aligning with the project's convention:

requests_mock>=1.11,<2

@mvarendorff mvarendorff marked this pull request as draft April 1, 2026 06:43
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.

4 participants