Rewrite sqlfluff integration tests#22099
Open
grihabor wants to merge 1 commit intopantsbuild:mainfrom
Open
Conversation
Integration tests for sqlfluff are not true integration tests right now. They call LintRequest rules, but that skips the partitioning step. I need to test partitioning in pantsbuild#21937, so I had to rewrite the tests.
huonw
reviewed
Apr 15, 2025
Contributor
huonw
left a comment
huonw
left a comment
There was a problem hiding this comment.
Thanks for maintaining this backend.
Switching to run_pants makes the tests quite a bit slower: they're now hitting the 60s timeout on CI, and a n=1 test goes from 50s to 104s locally.
I think it would be good to consider a middle ground, where:
- most tests use the lighter-weight direct invocation of rules (most of our
*_integration_test.pyfiles do this even for integration tests, I think the rule of thumb is an "integration test" uses a scheduler and invokes rules, even with some mocking, whereas non-integration tests just invoke functions without needing a scheduler) - a few tests do full
run_pantswhere required (such as those that need to test partitioning etc. end to end)
For testing partitioning, I think one can also test the partitioning rules directly, to handle all the subtleties, and have one or two run_pants tests that suggest it's all hooked up correctly.
What do you think?
huonw
added
the
release-notes:not-required
Contributor
Author
|
@huonw Thank you for review! Well, I'd like to keep the integration tests, but if it's too slow I'm ok to switch to only rule based tests |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Integration tests for sqlfluff are not true integration tests right now. They call LintRequest rules, but that skips the partitioning step. I need to test partitioning in #21937, so I had to rewrite the tests.