-
Notifications
You must be signed in to change notification settings - Fork 6.6k
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
Add test cases for SelectStatementContext.findColumnProjection() #28843
base: master
Are you sure you want to change the base?
Conversation
112a985
to
861687d
Compare
Hi @Zhenye-Na, does this pr has any update? |
Hi @strongduanmu , I have lost the track of this PR for a while, thanks for reminding me of this un-finished work. From the previous checkpoint, I noticed that |
Hi @strongduanmu , Whenever you've got a chance, could you take a lookat this PR? If everything looks good, I'm happy to merge this in. |
@Test | ||
void assertFindColumnProjectionWithoutExpandProjections() { | ||
SelectStatementContext selectStatementContext = mock(SelectStatementContext.class, RETURNS_DEEP_STUBS); | ||
|
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.
Please remove useless blank line.
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.
Ack, I ran spotless:apply
so I thought the formatting should be good.
Is this something that we could potentially consider move into formatter rules or maybe this is the conventional code style that we used here?
|
||
assertFalse(result.isPresent()); | ||
} | ||
|
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.
Is it possible to add an unit test under normal scenarios?
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.
I tried with mocking, and was getting NPE after it, so I was considering at least getting the unit test for failed scenarios. If you prefer, I can try to get the successful cases in unit tests as well. Though I dont have a specific timeline for this.
Notes
This PR fixes #26467
Changes proposed in this pull request:
SelectStatementContext.findColumnProjection()
Testing
Checkstyle
Before committing this PR, I'm sure that I have checked the following options:
./mvnw clean install -B -T1C -Dmaven.javadoc.skip -Dmaven.jacoco.skip -e
.