kingswanwho commented on code in PR #2499:
URL: https://github.com/apache/drill/pull/2499#discussion_r1094240171
##########
exec/java-exec/src/test/java/org/apache/drill/test/ClusterTest.java:
##########
@@ -125,4 +132,56 @@ public static void run(String query, Object... args)
throws Exception {
public QueryBuilder queryBuilder( ) {
return client.queryBuilder();
}
+
+ /**
+ * Utility method which tests given query produces a {@link UserException}
and the exception message contains
+ * the given message.
+ * @param testSqlQuery Test query
+ * @param expectedErrorMsg Expected error message.
+ */
+ protected static void errorMsgTestHelper(String testSqlQuery, String
expectedErrorMsg) throws Exception {
+ try {
+ run(testSqlQuery);
+ fail("Expected a UserException when running " + testSqlQuery);
+ } catch (UserException actualException) {
+ try {
+ assertThat("message of UserException when running " + testSqlQuery,
actualException.getMessage(), containsString(expectedErrorMsg));
+ } catch (AssertionError e) {
+ e.addSuppressed(actualException);
+ throw e;
+ }
+ }
+ }
+
+ protected static void updateClient(Properties properties) {
+ if (client != null) {
+ client.close();
+ client = null;
+ }
+ ClientFixture.ClientBuilder clientBuilder = cluster.clientBuilder();
+ if (properties != null) {
+ for (final String key : properties.stringPropertyNames()) {
+ final String lowerCaseKey = key.toLowerCase();
+ clientBuilder.property(lowerCaseKey, properties.getProperty(key));
+ }
+ }
+ client = clientBuilder.build();
+ }
+
+ protected static void updateClient(final String user) {
+ updateClient(user, null);
+ }
+
+ protected static void updateClient(final String user, final String password)
{
+ if (client != null) {
+ client.close();
+ client = null;
+ }
+ final Properties properties = new Properties();
+ properties.setProperty(DrillProperties.USER, user);
+ if (password != null) {
+ properties.setProperty(DrillProperties.PASSWORD, password);
+ }
+ updateClient(properties);
+ }
Review Comment:
> Let's move these additions to ClientFixture. It will mean that they're no
longer drop-in replacements but they'll be more at home there.
Hi James, ClientFixture doesn't have a client variable there, so that we
could't close client at first and we need to change updateClient signature from
void to return ClientFixture, besides due to cluster variable here is none
static, we also need to change updateClient from static method to none static
method. Furthermore, there are still lots of test cases that we could just
update client without restart cluster, and for those cases that do need restart
cluster, we could use startCluster method to config new cluster and client at
same time without using updateClient. So maybe we could still keep those code
here?
--
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
