Skip to content
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

move some tests out of context and into sql #1846

Merged
merged 3 commits into from
Feb 17, 2022

Conversation

alamb
Copy link
Contributor

@alamb alamb commented Feb 16, 2022

Which issue does this PR close?

re #743
re #348

Rationale for this change

There are a bunch of tests in execution/context.rs that are not related to ExecutionContext (they are end to end tests for sql that happen to use the functions in ExecutionContext)

This both slows down compile times in the datafusion crate as well as makes it harder to discover existing test coverage.

This was annoying me recently when working in the context module so I decided to start moving the code over incrementally

What changes are included in this PR?

Move a test

Are there any user-facing changes?

No

@github-actions github-actions bot added ballista datafusion Changes in the datafusion crate labels Feb 16, 2022
@alamb alamb force-pushed the alamb/test_ballista branch from 8943b4f to 464026d Compare February 16, 2022 20:22
@@ -1399,79 +1396,6 @@ mod tests {
Ok(())
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

context.rs is still 3300 lines ,but it is getting better lol

@alamb alamb merged commit 4c0b17f into apache:master Feb 17, 2022
@alamb alamb deleted the alamb/test_ballista branch February 17, 2022 12:07
@alamb
Copy link
Contributor Author

alamb commented Feb 17, 2022

Thank you for the review @xudong963

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
datafusion Changes in the datafusion crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants