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
[BUGFIX] Respect test backends #4287
Conversation
✔️ Deploy Preview for niobium-lead-7998 ready! 🔨 Explore the source changes: a713548 🔍 Inspect the deploy log: https://app.netlify.com/sites/niobium-lead-7998/deploys/621d42c55e16e700082f22a7 😎 Browse the preview: https://deploy-preview-4287--niobium-lead-7998.netlify.app |
HOWDY! This is your friendly 🤖 CHANGELOG bot 🤖Please don't forget to add a clear and succinct description of your change under the Develop header in ✨ Thank you! ✨ |
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.
Can you please include a description of the bug/use-case and how this change addresses it?
|
||
# Some Expectations (mostly contrib) explicitly list test_backends/dialects to test with | ||
if d.test_backends: | ||
for tb in d.test_backends: | ||
engines_to_include[tb["backend"]] = True |
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.
Can ExpectationTestDataCases
be improved here with additional types and typing? Especially for test_backends
where there are clearly expected keys of "backend"
and "dialects"
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.
Do you have any suggestions for implementing?
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.
yes, introduce a type called TestBackend
that includes two fields, backend
and dialect
, then have the test_backends
property of ExpectationTestDataCases
return an Optional[List[TestBackend]]
|
||
# Some Expectations (mostly contrib) explicitly list test_backends/dialects to test with | ||
if d.test_backends: | ||
for tb in d.test_backends: | ||
engines_to_include[tb["backend"]] = True |
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.
yes, introduce a type called TestBackend
that includes two fields, backend
and dialect
, then have the test_backends
property of ExpectationTestDataCases
return an Optional[List[TestBackend]]
Changes proposed in this pull request:
Definition of Done
Please delete options that are not relevant.