-
Notifications
You must be signed in to change notification settings - Fork 70
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
✨ feat(gentest): use jinja2 templating in gentest
#900
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Looks like a great start to adding jinja2 templating!
I know it wasn't the focus of your PR, but I couldn't help but try and improve the output a little. The suggestions below should ensure that the generated output passes flake8/isort/mypy checks, but I might have missed something :)
Co-authored-by: danceratopz <[email protected]>
Co-authored-by: danceratopz <[email protected]>
Co-authored-by: danceratopz <[email protected]>
Co-authored-by: danceratopz <[email protected]>
Co-authored-by: danceratopz <[email protected]>
Co-authored-by: danceratopz <[email protected]>
Co-authored-by: danceratopz <[email protected]>
Co-authored-by: danceratopz <[email protected]>
I passed |
Got it! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for including the suggestions!
gentest
to Jinja2gentest
🗒️ Description
This PR introduces the use of Jinja2 templates for generating test scripts, contained within a new
gentest
package that includes self-contained templates and related modules.Notes on code refactoring
The code has been refactored for simplicity. For instance, the previous implementation used a
RemoteTransaction
dataclass:This required nested calls like
tr.transaction
:execution-spec-tests/src/cli/gentest.py
Line 128 in 6fb195b
The class has been flattened by making it inherit from the
Transaction
type, which now provides Pydantic validations out of the box:source
This results in a simpler interface:
source
More importantly, pydantic rightly identified that
to
field of transaction can be null which is now handled:source
The
test_providers
module provides a predictable interface that provides context for the templates and is designed to play nice with unit testing.Future Improvements
The generated tests can be further improved later. This PR focuses solely on the migration process, allowing for testing by diffing the generated tests against those from the main branch.
🔗 Related Issues
closes #863
✅ Checklist
mkdocs serve
locally and verified the auto-generated docs for new tests in the Test Case Reference are correctly formatted.