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

Refactor tests into separate functions that expect errors or not #123

Merged
merged 4 commits into from
Aug 3, 2020

Conversation

rogerlucena
Copy link
Contributor

For clarity, it is useful to not mix in the same test function cases for which it is expected an error and cases for which it is not.

Separating these cases into two different test functions makes the code cleaner and avoids the unnecessary use of more complicated ifs inside the body of the test function. Also, it makes it easier to add new test cases in the future as it is simpler to realize where they belong to in the test file.

Then, this PR comes to follow this guideline for two important test functions inside expression_test.go and planner_test.go.

@thiagovas thiagovas self-requested a review July 30, 2020 20:57
bql/semantic/expression_test.go Show resolved Hide resolved
bql/planner/planner_test.go Show resolved Hide resolved
bql/planner/planner_test.go Show resolved Hide resolved
Copy link

@Tati1701 Tati1701 left a comment

Choose a reason for hiding this comment

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

Thanks, Roger! LGTM! :D

bql/planner/planner_test.go Outdated Show resolved Hide resolved
@rbkloss rbkloss merged commit f6d3928 into google:master Aug 3, 2020
@rogerlucena rogerlucena deleted the refactor-tests branch August 3, 2020 19:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants