[ https://issues.apache.org/jira/browse/DRILL-8117?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17683276#comment-17683276 ]
ASF GitHub Bot commented on DRILL-8117: --------------------------------------- kingswanwho commented on code in PR #2499: URL: https://github.com/apache/drill/pull/2499#discussion_r1094205201 ########## 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: HI James, ClientFixture doesn't have client variable, so that we could't close client at first. And cluster here is a non-static variable, we should change all updateClient static method to non-static method, should we still keep changing this? ########## 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: Hi James, ClientFixture doesn't have client variable, so that we could't close client at first. And cluster here is a non-static variable, we should change all updateClient static method to non-static method, should we still keep changing this? > Upgrade unit tests to the cluster fixture framework > --------------------------------------------------- > > Key: DRILL-8117 > URL: https://issues.apache.org/jira/browse/DRILL-8117 > Project: Apache Drill > Issue Type: Improvement > Affects Versions: 1.20.1 > Reporter: Jingchuan Hu > Assignee: James Turton > Priority: Major > Fix For: 1.21.0 > > > Upgrade various unit tests to the cluster fixture framework and replace other > instances of deprecated code usage. -- This message was sent by Atlassian Jira (v8.20.10#820010)