Add Python translation with pytest parametrization #1
Conversation
…s, and duplicated tests
There was a problem hiding this comment.
Pull Request Overview
Adds a full Python translation illustrating pytest parametrization and implements a simple Order/Status kata with corresponding tests, plus basic project setup.
- Introduces extensive
test_parametrized_examples.pydemonstrating various pytest features. - Implements
OrderandStatusclasses with state-based tests intest_order_state.py. - Provides project config, sample CSV data, readmes, and entry-point setup.
Reviewed Changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| python/tests/test_parametrized_examples.py | Adds comprehensive pytest parametrization examples |
| python/tests/test_order_state.py | Introduces order state unit tests (duplicated cases) |
| python/tests/test_data.csv | Provides sample CSV data for file-based parametrization |
| python/tests/README.md | Documents parametrization examples and how to run them |
| python/src/coffeeshop_kata/status.py | Defines Status enum for order states |
| python/src/coffeeshop_kata/order.py | Implements Order class with status/change-permission logic |
| python/pyproject.toml | Sets project metadata, Python requirements, and pytest dependency |
| python/main.py | Adds a simple entry-point printing message |
| python/README.md | Describes setup and test commands |
| python/.python-version | Pins Python version |
| python/.gitignore | Adds Python and IDE ignore patterns |
Comments suppressed due to low confidence (1)
python/tests/README.md:55
- [nitpick] The path
tests/test_parametrized_examples.pymay be confusing if the working directory ispython/; consider clarifying the expected CWD or adjusting the path.
uv run pytest tests/test_parametrized_examples.py -v
- Run all tests except expected failing test - Run expected failing test separately, expecting it to fail - Matches Java CI pattern but as separate workflow 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
|
Thank you so much @nitsanavni for the PR. I will review it immediately. |
|
|
||
| - name: all tests except the expected failing one | ||
| working-directory: python | ||
| run: uv run pytest -k "not test_order_cannot_be_updated_or_canceled_if_paid" -v |
There was a problem hiding this comment.
The Java workflow calls maven and checks if the test that is expect to fail really did fail.
Unfortunately, doing this is not really nicely possible from the code (ignoring the mvn failure and grep in the surefire output).
Here in python only the non-failing tests are run, so we don't know if the test that should fail in the build actually failed.
Would it be possible to do this in the python build as well? Would be nice, but is not a must.
There was a problem hiding this comment.
So that's just a CI-nice-to-have-issue and has nothing to do with the content itself.
There was a problem hiding this comment.
we do both, next step of the CI checks that the failing test is failing
There was a problem hiding this comment.
There was a problem hiding this comment.
Ah, I think now I got it: test_order_cannot_be_updated_or_canceled_if_paid is the failing test, right? So you are executing exactly the failing test and the exclamation mark negates the rc of the command so the command is successful if it failed, is that what the line does?
- Only trigger on python.yml changes, not all workflows - Push: main branch only - PR: targeting main branch only 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
|
@pfichtner shall we merge? |
Of course! :) Thanks so much for contributing @nitsanavni! |
| def load_csv_data() -> list[tuple[str, str, int]]: | ||
| """Load test data from CSV file.""" | ||
| csv_path = Path(__file__).parent / "test_data.csv" | ||
| if not csv_path.exists(): |
There was a problem hiding this comment.
@nitsanavni just removed the fallback which is very weird that AI generated such fallback code in a test
No description provided.