kingswanwho commented on code in PR #2499:
URL: https://github.com/apache/drill/pull/2499#discussion_r968518305
##########
exec/java-exec/src/test/java/org/apache/drill/exec/impersonation/TestInboundImpersonation.java:
##########
@@ -156,22 +159,25 @@ public void unauthorizedTarget() throws Exception {
@Test
public void invalidPolicy() throws Exception {
- thrownException.expect(new
UserExceptionMatcher(UserBitShared.DrillPBError.ErrorType.VALIDATION,
- "Invalid impersonation policies."));
+ String query = "ALTER SYSTEM SET `%s`='%s'";
Review Comment:
Hi James, the **_client_** here inherits from BaseTestQuery, which is
DrillClient but not ClientFixture in TestLargeFileCompilation as Paul
suggested. DrillClient doesn't have method like client.alterSystem().
BaseTestQuery is deprecated actually, but not marked as deprecated because it
is still widely used. The best choice here I think is change base test class
from BaseTestQuery to ClusterTest so that we could use client.alterSystem() for
clean and compact code style. However there are multiple Inheritance levels
here, which are TestInboundImpersonation -> BaseTestImpersonation ->
PlanTestBase -> BaseTestQuery. Due to that PlanTestBase involves many classes.
Maybe change BaseTestImpersonation inheritance from PlanTestBase to ClusterTest
directly is an easier way.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@drill.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
