zabetak commented on code in PR #6225:
URL: https://github.com/apache/hive/pull/6225#discussion_r2592046067
##########
itests/hive-unit/src/test/java/org/apache/hive/beeline/TestHplSqlViaBeeLine.java:
##########
@@ -1198,9 +1196,7 @@ public void testHplSqlProcedureWithSYSDATEUdf() throws
Throwable {
"END;\n" +
"p1('Bob');\n" +
"SELECT * FROM result;" ;
Review Comment:
I feel that the SELECT **all** pattern along with validating all columns
("Bob") adds an extra layer of complexity that is not really required for these
tests that are focusing on DATETIME functions. Personally, I would change the
query to
```sql
SELECT join_date FROM result
```
and drop all complex logic of composite patterns.
##########
itests/hive-unit/src/test/java/org/apache/hive/beeline/TestHplSqlViaBeeLine.java:
##########
@@ -1411,8 +1404,174 @@ private void testScriptFile(String scriptText,
List<String> argList, String expe
testScriptFile(scriptText, argList, expectedPattern, OutStream.OUT);
}
+ /**
+ * Checks that the pattern appears in the output.
+ * <p><b>Warning:</b> If the output depends on the time of the execution,
+ * use {@link #testTimeScriptFile}. This may be the case if
+ * the script contains CURRENT_DATE, CURRENT_TIME_MILLIS, etc.</p>
+ */
private void testScriptFile(String scriptText, List<String> argList, String
expectedPattern,
TestBeeLineWithArgs.OutStream outStream) throws Throwable {
+ String output = runScriptFile(scriptText, argList, outStream);
+ if (!Pattern.compile(".*" + expectedPattern + ".*",
Pattern.DOTALL).matcher(output).matches()) {
+ fail("Output: '" + output + "' should match " + expectedPattern);
+ }
+ }
+
+ interface TimeCheckHelper {
Review Comment:
If we drop the `withPrefixPattern` method we could turn this type-safe enum
pattern (via `interface`) to an actual `enum`.
##########
itests/hive-unit/src/test/java/org/apache/hive/beeline/TestHplSqlViaBeeLine.java:
##########
@@ -1411,8 +1404,174 @@ private void testScriptFile(String scriptText,
List<String> argList, String expe
testScriptFile(scriptText, argList, expectedPattern, OutStream.OUT);
}
+ /**
+ * Checks that the pattern appears in the output.
+ * <p><b>Warning:</b> If the output depends on the time of the execution,
+ * use {@link #testTimeScriptFile}. This may be the case if
+ * the script contains CURRENT_DATE, CURRENT_TIME_MILLIS, etc.</p>
+ */
private void testScriptFile(String scriptText, List<String> argList, String
expectedPattern,
TestBeeLineWithArgs.OutStream outStream) throws Throwable {
+ String output = runScriptFile(scriptText, argList, outStream);
+ if (!Pattern.compile(".*" + expectedPattern + ".*",
Pattern.DOTALL).matcher(output).matches()) {
+ fail("Output: '" + output + "' should match " + expectedPattern);
+ }
+ }
+
+ interface TimeCheckHelper {
+
+ /** Supports timestamps in second resolution such as 1764680360 */
+ TimeCheckHelper TIMESTAMP_SECONDS = new TimeCheckHelper() {
+ // works for timestamps a few hours after the epoch (1970-01-01 00:00:00
GMT)
+ Pattern PATTERN = Pattern.compile("[0-9]{5,}");
+
+ @Override
+ public long getCurrentTime() {
+ return System.currentTimeMillis() / 1000;
+ }
+
+ @Override
+ public long strToTime(String str) {
+ return Long.parseLong(str);
+ }
+
+ @Override
+ public Pattern getPattern() {
+ return PATTERN;
+ }
+ };
+
+ /** Supports timestamps in millisecond resolution such as 1764680360883 */
+ TimeCheckHelper TIMESTAMP_MILLISECONDS = new TimeCheckHelper() {
+ Pattern PATTERN = Pattern.compile("[0-9]{5,}");
+
+ @Override
+ public long getCurrentTime() {
+ return System.currentTimeMillis();
+ }
+
+ @Override
+ public long strToTime(String str) {
+ return Long.parseLong(str);
+ }
+
+ @Override
+ public Pattern getPattern() {
+ return PATTERN;
+ }
+ };
+
+ /** Supports dates such as '2025-12-02' */
+ TimeCheckHelper DATE = new TimeCheckHelper() {
+ Pattern PATTERN = Pattern.compile("[0-9]{4}-[0-9]{1,2}-[0-9]{1,2}");
+
+ @Override
+ public long getCurrentTime() {
+ Date date = new Date(System.currentTimeMillis());
+ return strToTime(date.toString());
+ }
+
+ @Override
+ public long strToTime(String str) {
+ return Date.valueOf(str).getTime();
+ }
+
+ @Override
+ public Pattern getPattern() {
+ return PATTERN;
+ }
+ };
+
+ /** Supports datetimes such as '2025-12-02 02:56:08' */
+ TimeCheckHelper DATETIME = new TimeCheckHelper() {
+ Pattern PATTERN = Pattern.compile("[0-9]{4}-[0-9]{1,2}-[0-9]{1,2}
+[0-9]{2}:[0-9]{2}:[0-9]{2}");
+
+ @Override
+ public long getCurrentTime() {
+ return System.currentTimeMillis() / 1000;
+ }
+
+ @Override
+ public long strToTime(String str) {
+ return Timestamp.valueOf(str).getTime() / 1000;
+ }
+
+ @Override
+ public Pattern getPattern() {
+ return PATTERN;
+ }
+ };
+
+ /** The current time. The resolution (e.g., milliseconds) must be the same
as {@link #strToTime(String)}. */
+ long getCurrentTime();
+
+ /**
+ * Converts a candidate match to a time. The resolution (e.g.,
milliseconds) must be the same
+ * as {@link #getCurrentTime()}. */
+ long strToTime(String str);
+
+ /** A pattern to find string representations of the time in the output. */
+ Pattern getPattern();
+
+ /** Limits the candidates to the ones that match prefixPattern followed by
this pattern. */
+ default TimeCheckHelper withPrefixPattern(Pattern prefixPattern) {
+ TimeCheckHelper parent = this;
+ return new TimeCheckHelper() {
+ @Override
+ public long getCurrentTime() {
+ return parent.getCurrentTime();
+ }
+
+ @Override
+ public long strToTime(String str) {
+ Matcher m = parent.getPattern().matcher(str);
+ if (!m.find(0)) {
+ throw new IllegalStateException("String does not contain a match
for the pattern " + parent.getPattern()
+ .pattern() + ":\nThe string was: " + str);
+ }
+ return parent.strToTime(str.substring(m.start()));
+ }
+
+ @Override
+ public Pattern getPattern() {
+ return Pattern.compile(prefixPattern +
parent.getPattern().pattern());
+ }
+
+ @Override
+ public TimeCheckHelper withPrefixPattern(Pattern prefixPattern) {
+ throw new UnsupportedOperationException();
+ }
+ };
+ }
+ }
+
+ /** Checks that the output of the script mentions a time as defined by the
helper. */
+ private void testTimeScriptFile(String scriptText, List<String> argList,
TestBeeLineWithArgs.OutStream outStream,
Review Comment:
I think it would nicer to have this as an overload:
```java
private void testScriptFile(String scriptText, List<String> argList,
TimeCheckHelper checker)
```
##########
itests/hive-unit/src/test/java/org/apache/hive/beeline/TestHplSqlViaBeeLine.java:
##########
@@ -1411,8 +1404,174 @@ private void testScriptFile(String scriptText,
List<String> argList, String expe
testScriptFile(scriptText, argList, expectedPattern, OutStream.OUT);
}
+ /**
+ * Checks that the pattern appears in the output.
+ * <p><b>Warning:</b> If the output depends on the time of the execution,
+ * use {@link #testTimeScriptFile}. This may be the case if
+ * the script contains CURRENT_DATE, CURRENT_TIME_MILLIS, etc.</p>
+ */
private void testScriptFile(String scriptText, List<String> argList, String
expectedPattern,
TestBeeLineWithArgs.OutStream outStream) throws Throwable {
+ String output = runScriptFile(scriptText, argList, outStream);
+ if (!Pattern.compile(".*" + expectedPattern + ".*",
Pattern.DOTALL).matcher(output).matches()) {
+ fail("Output: '" + output + "' should match " + expectedPattern);
+ }
+ }
+
+ interface TimeCheckHelper {
+
+ /** Supports timestamps in second resolution such as 1764680360 */
+ TimeCheckHelper TIMESTAMP_SECONDS = new TimeCheckHelper() {
+ // works for timestamps a few hours after the epoch (1970-01-01 00:00:00
GMT)
+ Pattern PATTERN = Pattern.compile("[0-9]{5,}");
+
+ @Override
+ public long getCurrentTime() {
+ return System.currentTimeMillis() / 1000;
+ }
+
+ @Override
+ public long strToTime(String str) {
+ return Long.parseLong(str);
+ }
+
+ @Override
+ public Pattern getPattern() {
+ return PATTERN;
+ }
+ };
+
+ /** Supports timestamps in millisecond resolution such as 1764680360883 */
+ TimeCheckHelper TIMESTAMP_MILLISECONDS = new TimeCheckHelper() {
+ Pattern PATTERN = Pattern.compile("[0-9]{5,}");
+
+ @Override
+ public long getCurrentTime() {
+ return System.currentTimeMillis();
+ }
+
+ @Override
+ public long strToTime(String str) {
+ return Long.parseLong(str);
+ }
+
+ @Override
+ public Pattern getPattern() {
+ return PATTERN;
+ }
+ };
+
+ /** Supports dates such as '2025-12-02' */
+ TimeCheckHelper DATE = new TimeCheckHelper() {
+ Pattern PATTERN = Pattern.compile("[0-9]{4}-[0-9]{1,2}-[0-9]{1,2}");
+
+ @Override
+ public long getCurrentTime() {
+ Date date = new Date(System.currentTimeMillis());
+ return strToTime(date.toString());
+ }
+
+ @Override
+ public long strToTime(String str) {
+ return Date.valueOf(str).getTime();
+ }
+
+ @Override
+ public Pattern getPattern() {
+ return PATTERN;
+ }
+ };
+
+ /** Supports datetimes such as '2025-12-02 02:56:08' */
+ TimeCheckHelper DATETIME = new TimeCheckHelper() {
+ Pattern PATTERN = Pattern.compile("[0-9]{4}-[0-9]{1,2}-[0-9]{1,2}
+[0-9]{2}:[0-9]{2}:[0-9]{2}");
+
+ @Override
+ public long getCurrentTime() {
+ return System.currentTimeMillis() / 1000;
+ }
+
+ @Override
+ public long strToTime(String str) {
+ return Timestamp.valueOf(str).getTime() / 1000;
+ }
+
+ @Override
+ public Pattern getPattern() {
+ return PATTERN;
+ }
+ };
+
+ /** The current time. The resolution (e.g., milliseconds) must be the same
as {@link #strToTime(String)}. */
+ long getCurrentTime();
+
+ /**
+ * Converts a candidate match to a time. The resolution (e.g.,
milliseconds) must be the same
+ * as {@link #getCurrentTime()}. */
+ long strToTime(String str);
+
+ /** A pattern to find string representations of the time in the output. */
+ Pattern getPattern();
+
+ /** Limits the candidates to the ones that match prefixPattern followed by
this pattern. */
+ default TimeCheckHelper withPrefixPattern(Pattern prefixPattern) {
Review Comment:
As mentioned previously, the additional complexity here may not be necessary
if we modify slightly the test cases and make them more focused on the actual
goal of the test.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
