-
Notifications
You must be signed in to change notification settings - Fork 73
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: improve included tap and target tests in singer_sdk.testing
#1171
Conversation
Todo:
|
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.
@kgpayne Just some final nits and this will be g2g 😄
@kgpayne - Fantastic work here. I'll try to do a final review tonight. |
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.
This is great stuff, Ken!
@kgpayne - I want to just confirm this doesn't break tests on existing taps and targets. Do you know? I updated this one to confirm on the target side: ✅ Tests pass above after reverting the original test file as And we probably should do similar on a tap implementation is possible. |
@aaronsteers yes, this is backwards compatible 🙂 The new factory functions have new names, and work differently, so are opt-in. @edgarrmondragon also asked, and I did some spot checks to be extra sure 👍 |
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.
@kgpayne - Thanks for all your time today processing my questions in our zoom call. Based on our conversation, I think this meets the criteria of a stable increment and is ready to ship.
@edgarrmondragon - I know you put tons of effort and time into this as well, so thank you. 🙏
Follow ups identified in our call (will edit later to add links):
These are non-blocking for this PR, as discussed. Developers may run into scaling limits if their config defines a very large test dataset, but existing tests will still work regardless, and the failures / scaling issues will be raised only when the developer is in a mode to expect and deal with those. |
Closes #1169
singer_sdk.testing
#1169This work incorporates both #580 and #1123 into a unified testing framework for developers tap and target implementations.
Example usage with sample
TapCountries
:Produces the following
pytest -v
output:and the following UI in VSCode:
📚 Documentation preview 📚: https://meltano-sdk--1171.org.readthedocs.build/en/1171/