Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create pytest compatible tests for existing tests. #186

Merged
merged 11 commits into from
Feb 27, 2024

Conversation

pcdeadeasy
Copy link
Contributor

@pcdeadeasy pcdeadeasy commented Feb 21, 2024

  • Create test_* files that invoke the classes from other modules and run the python to typescript schema generator.
  • TBD, add a way to do snapshot testing
  • Invoke the tests as part of the CI/CD pipeline

You can run the tests a couple of ways:

cd python/tests
pytest

or

cd python
hatch run test

@pcdeadeasy pcdeadeasy changed the title Create pytest compatible tests for existing test. Create pytest compatible tests for existing tests. Feb 21, 2024
Comment on lines 4 to 9
def test_python_to_typescript_schema():
result = python_type_to_typescript_schema(Cart)
assert result is not None
assert result.typescript_type_reference == "Cart"
assert result.typescript_schema_str is not None
assert not result.errors
Copy link
Member

Choose a reason for hiding this comment

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

So I'm not entirely familiar with it, but if we add syrupy, it looks like you can add a parameter called snapshot and you can just write

Suggested change
def test_python_to_typescript_schema():
result = python_type_to_typescript_schema(Cart)
assert result is not None
assert result.typescript_type_reference == "Cart"
assert result.typescript_schema_str is not None
assert not result.errors
def test_coffeeshop(snapshot):
result = python_type_to_typescript_schema(Cart)
assert result == snapshot

Let me know how that works

Copy link
Contributor Author

@pcdeadeasy pcdeadeasy Feb 24, 2024

Choose a reason for hiding this comment

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

@DanielRosenwasser Worked out really well. Except it looks like the function python_type_to_typescript_schema produced the python to typescript conversions in an order that could change. Look at the outputs:

assert [+ received] == [- snapshot]
E - TypeScriptSchemaConversionResult(typescript_schema_str='interface Cart {\n type: "Cart";\n items: Array<LineItem | UnknownText>;\n}\n\ninterface LineItem {\n type: "LineItem";\n product: BakeryProduct | LatteDrink | CoffeeDrink | EspressoDrink | UnknownText;\n quantity: number;\n}\n\ninterface LatteDrink {\n type: "LatteDrink";\n name: "cappuccino" | "flat white" | "latte" | "latte macchiato" | "mocha" | "chai latte";\n temperature?: "hot" | "extra hot" | "warm" | "iced";\n // The default is \'grande\'\n size?: "short" | "tall" | "grande" | "venti";\n options?: Array<Creamer | Sweetener | Syrup | Topping | Caffeine | LattePreparation>;\n}\n\ninterface CoffeeDrink {\n type: "CoffeeDrink";\n name: "americano" | "coffee";\n temperature?: "hot" | "extra hot" | "warm" | "iced";\n // The default is \'grande\'\n size?: "short" | "tall" | "grande" | "venti";\n options?: Array<Creamer | Sweetener | Syrup | Topping | Caffeine | LattePreparation>;\n}\n\ninterface BakeryProduct {\n type: "BakeryProduct";\n name: "apple bran muffin" | "blueberry muffin" | "lemon poppyseed muffin" | "bagel";\n options?: Array<BakeryOption | BakeryPreparation>;\n}\n\ninterface Topping {\n type: "Topping";\n name: "cinnamon" | "foam" | "ice" | "nutmeg" | "w...

E + TypeScriptSchemaConversionResult(typescript_schema_str='interface Cart {\n type: "Cart";\n items: Array<LineItem | UnknownText>;\n}\n\ninterface LineItem {\n type: "LineItem";\n product: BakeryProduct | LatteDrink | CoffeeDrink | EspressoDrink | UnknownText;\n quantity: number;\n}\n\ninterface BakeryProduct {\n type: "BakeryProduct";\n name: "apple bran muffin" | "blueberry muffin" | "lemon poppyseed muffin" | "bagel";\n options?: Array<BakeryOption | BakeryPreparation>;\n}\n\ninterface BakeryPreparation {\n type: "BakeryPreparation";\n name: "warmed" | "cut in half";\n}\n\n// Represents any text that could not be understood.\ninterface UnknownText {\n type: "UnknownText";\n // The text that wasn\'t understood\n text: string;\n}\n\ninterface CoffeeDrink {\n type: "CoffeeDrink";\n name: "americano" | "coffee";\n temperature?: "hot" | "extra hot" | "warm" | "iced";\n // The default is \'grande\'\n size?: "short" | "tall" | "grande" | "venti";\n options?: Array<Creamer | Sweetener | Syrup | Topping | Caffeine | LattePreparation>;\n}\n\ninterface LattePreparation {\n type: "LattePreparation";\n name: "for here cup" | "lid" | "with room" | "to go" | "dry" | "wet";\n}\n\ninterface LatteDrink {\n type: "LatteDrink";\n name: "cap...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As a result the snapshot tests fail. Once we can make the output more deterministic this will work out great.

Copy link
Member

Choose a reason for hiding this comment

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

I'm extremely surprised - you're saying across runs the output changes?

Copy link
Member

Choose a reason for hiding this comment

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

Duh, sets and dicts don't specify an iteration order in Python - they are guaranteed to have insertion order in JavaScript.

Copy link
Member

Choose a reason for hiding this comment

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

I'll fix that by Monday.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No worries. The other is test failing was the hello world one but I guess all of them might have the same issue. We need to maintain the order to be able to run the snapshot tests.

Copy link
Member

Choose a reason for hiding this comment

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

#192 should fix the instances in the Py->TS translation where order matters on sets and dicts

@pcdeadeasy
Copy link
Contributor Author

@DanielRosenwasser and @hillary-mutisya, could you review. With Daniel's last PR, we now have snapshot testing enabled. If tests change, the snapshots will need to be regenerated. (pytest --snapshot-update)

Copy link
Member

@DanielRosenwasser DanielRosenwasser left a comment

Choose a reason for hiding this comment

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

Could you inline the schemas into each test file?

Also, it's a little unfortunate that the baseline output is so unreadable, but we can fix this later with some customization in syrupy.

python/tests/test_generic_alias_2.py Outdated Show resolved Hide resolved
python/tests/test_generric_alias_1.py Outdated Show resolved Hide resolved
python/tests/test_generric_alias_1.py Outdated Show resolved Hide resolved
Copy link
Member

@DanielRosenwasser DanielRosenwasser left a comment

Choose a reason for hiding this comment

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

Could you inline the schemas into each test file?

Also, it's a little unfortunate that the baseline output is so unreadable, but we can fix this later with some customization in syrupy.

@pcdeadeasy pcdeadeasy marked this pull request as ready for review February 27, 2024 18:47
@DanielRosenwasser DanielRosenwasser merged commit 2af0e6b into main Feb 27, 2024
8 checks passed
@DanielRosenwasser DanielRosenwasser deleted the create-pytests branch February 27, 2024 18:51
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.

2 participants