From 3c2452478cb089595fe0d6d4a37fdd4b224bef79 Mon Sep 17 00:00:00 2001 From: Sokwhan Huh Date: Thu, 30 Jan 2025 11:59:36 -0800 Subject: [PATCH] Fix an issue with the error message not including the source location and position during function dispatch PiperOrigin-RevId: 721471017 --- .../test/java/dev/cel/bundle/CelImplTest.java | 4 +- codelab/src/test/codelab/Exercise1Test.java | 2 +- codelab/src/test/codelab/Exercise3Test.java | 9 +- codelab/src/test/codelab/Exercise8Test.java | 2 +- .../test/codelab/solutions/Exercise1Test.java | 2 +- .../test/codelab/solutions/Exercise3Test.java | 9 +- .../test/codelab/solutions/Exercise8Test.java | 2 +- .../extensions/CelListsExtensionsTest.java | 2 +- .../CelEvaluationExceptionBuilder.java | 9 +- .../dev/cel/runtime/DefaultInterpreter.java | 8 +- .../java/dev/cel/runtime/DefaultMetadata.java | 7 +- .../main/java/dev/cel/runtime/Metadata.java | 3 + .../CelEvaluationExceptionBuilderTest.java | 5 + .../test/resources/arithmInt64_error.baseline | 16 ++-- .../resources/arithmUInt64_error.baseline | 12 +-- .../test/resources/boolConversions.baseline | 14 +-- .../resources/boolConversions_error.baseline | 11 +++ runtime/src/test/resources/booleans.baseline | 51 +--------- .../test/resources/booleans_error.baseline | 35 +++++++ .../test/resources/doubleConversions.baseline | 6 -- .../doubleConversions_error.baseline | 5 + .../test/resources/int64Conversions.baseline | 14 +-- .../resources/int64Conversions_error.baseline | 11 +++ runtime/src/test/resources/lists.baseline | 17 +--- .../src/test/resources/lists_error.baseline | 11 +++ .../test/resources/optional_errors.baseline | 4 +- .../resources/regexpMatches_error.baseline | 6 +- .../test/resources/timeConversions.baseline | 9 -- .../resources/timeConversions_error.baseline | 5 + .../test/resources/uint64Conversions.baseline | 20 +--- .../uint64Conversions_error.baseline | 17 ++++ .../test/resources/unknownResultSet.baseline | 8 +- .../dev/cel/testing/BaseInterpreterTest.java | 92 ++++++++++++------- .../DurationLiteralValidatorTest.java | 8 +- .../validators/RegexLiteralValidatorTest.java | 12 ++- .../TimestampLiteralValidatorTest.java | 7 +- 36 files changed, 246 insertions(+), 209 deletions(-) create mode 100644 runtime/src/test/resources/boolConversions_error.baseline create mode 100644 runtime/src/test/resources/booleans_error.baseline create mode 100644 runtime/src/test/resources/doubleConversions_error.baseline create mode 100644 runtime/src/test/resources/int64Conversions_error.baseline create mode 100644 runtime/src/test/resources/lists_error.baseline create mode 100644 runtime/src/test/resources/timeConversions_error.baseline create mode 100644 runtime/src/test/resources/uint64Conversions_error.baseline diff --git a/bundle/src/test/java/dev/cel/bundle/CelImplTest.java b/bundle/src/test/java/dev/cel/bundle/CelImplTest.java index 7fcb7de2..7b4ee408 100644 --- a/bundle/src/test/java/dev/cel/bundle/CelImplTest.java +++ b/bundle/src/test/java/dev/cel/bundle/CelImplTest.java @@ -2063,8 +2063,8 @@ public void program_regexProgramSizeExceedsLimit_throws() throws Exception { assertThat(e) .hasMessageThat() .contains( - "evaluation error: Regex pattern exceeds allowed program size. Allowed: 6, Provided:" - + " 7"); + "evaluation error at :13: Regex pattern exceeds allowed program size. Allowed:" + + " 6, Provided: 7"); assertThat(e.getErrorCode()).isEqualTo(CelErrorCode.INVALID_ARGUMENT); } diff --git a/codelab/src/test/codelab/Exercise1Test.java b/codelab/src/test/codelab/Exercise1Test.java index 6d7a1694..7ea1aa1e 100644 --- a/codelab/src/test/codelab/Exercise1Test.java +++ b/codelab/src/test/codelab/Exercise1Test.java @@ -68,6 +68,6 @@ public void evaluate_divideByZeroExpression_throwsEvaluationException() throws E IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, () -> exercise1.eval(ast)); - assertThat(exception).hasMessageThat().contains("evaluation error: / by zero"); + assertThat(exception).hasMessageThat().contains("evaluation error at :1: / by zero"); } } diff --git a/codelab/src/test/codelab/Exercise3Test.java b/codelab/src/test/codelab/Exercise3Test.java index 9c62d9ad..8bf0aa02 100644 --- a/codelab/src/test/codelab/Exercise3Test.java +++ b/codelab/src/test/codelab/Exercise3Test.java @@ -57,7 +57,8 @@ public void evaluate_logicalOrFailure_throwsException(String expression) { assertThat(exception).hasMessageThat().contains("Evaluation error has occurred."); assertThat(exception).hasCauseThat().isInstanceOf(CelEvaluationException.class); - assertThat(exception).hasCauseThat().hasMessageThat().contains("evaluation error: / by zero"); + assertThat(exception).hasCauseThat().hasMessageThat().contains("evaluation error"); + assertThat(exception).hasCauseThat().hasMessageThat().contains("/ by zero"); } @Test @@ -77,7 +78,8 @@ public void evaluate_logicalAndFailure_throwsException(String expression) { assertThat(exception).hasMessageThat().contains("Evaluation error has occurred."); assertThat(exception).hasCauseThat().isInstanceOf(CelEvaluationException.class); - assertThat(exception).hasCauseThat().hasMessageThat().contains("evaluation error: / by zero"); + assertThat(exception).hasCauseThat().hasMessageThat().contains("evaluation error"); + assertThat(exception).hasCauseThat().hasMessageThat().contains("/ by zero"); } @Test @@ -98,6 +100,7 @@ public void evaluate_ternaryFailure_throwsException(String expression) { assertThat(exception).hasMessageThat().contains("Evaluation error has occurred."); assertThat(exception).hasCauseThat().isInstanceOf(CelEvaluationException.class); - assertThat(exception).hasCauseThat().hasMessageThat().contains("evaluation error: / by zero"); + assertThat(exception).hasCauseThat().hasMessageThat().contains("evaluation error"); + assertThat(exception).hasCauseThat().hasMessageThat().contains("/ by zero"); } } diff --git a/codelab/src/test/codelab/Exercise8Test.java b/codelab/src/test/codelab/Exercise8Test.java index cda19443..9ebbc505 100644 --- a/codelab/src/test/codelab/Exercise8Test.java +++ b/codelab/src/test/codelab/Exercise8Test.java @@ -128,7 +128,7 @@ public void optimize_constantFold_evaluateError() throws Exception { .contains("evaluation error at :15: key 'referer' is not present in map."); assertThat(e2) .hasMessageThat() - .contains("evaluation error at :0: key 'referer' is not present in map."); + .contains("evaluation error: key 'referer' is not present in map."); } @Test diff --git a/codelab/src/test/codelab/solutions/Exercise1Test.java b/codelab/src/test/codelab/solutions/Exercise1Test.java index 8b01690d..c1e93862 100644 --- a/codelab/src/test/codelab/solutions/Exercise1Test.java +++ b/codelab/src/test/codelab/solutions/Exercise1Test.java @@ -68,6 +68,6 @@ public void evaluate_divideByZeroExpression_throwsEvaluationException() throws E IllegalArgumentException exception = assertThrows(IllegalArgumentException.class, () -> exercise1.eval(ast)); - assertThat(exception).hasMessageThat().contains("evaluation error: / by zero"); + assertThat(exception).hasMessageThat().contains("evaluation error at :1: / by zero"); } } diff --git a/codelab/src/test/codelab/solutions/Exercise3Test.java b/codelab/src/test/codelab/solutions/Exercise3Test.java index 47d1c347..0bef54de 100644 --- a/codelab/src/test/codelab/solutions/Exercise3Test.java +++ b/codelab/src/test/codelab/solutions/Exercise3Test.java @@ -57,7 +57,8 @@ public void evaluate_logicalOrFailure_throwsException(String expression) { assertThat(exception).hasMessageThat().contains("Evaluation error has occurred."); assertThat(exception).hasCauseThat().isInstanceOf(CelEvaluationException.class); - assertThat(exception).hasCauseThat().hasMessageThat().contains("evaluation error: / by zero"); + assertThat(exception).hasCauseThat().hasMessageThat().contains("evaluation error"); + assertThat(exception).hasCauseThat().hasMessageThat().contains("/ by zero"); } @Test @@ -83,7 +84,8 @@ public void evaluate_logicalAndFailure_throwsException(String expression) { assertThat(exception).hasMessageThat().contains("Evaluation error has occurred."); assertThat(exception).hasCauseThat().isInstanceOf(CelEvaluationException.class); - assertThat(exception).hasCauseThat().hasMessageThat().contains("evaluation error: / by zero"); + assertThat(exception).hasCauseThat().hasMessageThat().contains("evaluation error"); + assertThat(exception).hasCauseThat().hasMessageThat().contains("/ by zero"); } @Test @@ -108,6 +110,7 @@ public void evaluate_ternaryFailure_throwsException(String expression) { assertThat(exception).hasMessageThat().contains("Evaluation error has occurred."); assertThat(exception).hasCauseThat().isInstanceOf(CelEvaluationException.class); - assertThat(exception).hasCauseThat().hasMessageThat().contains("evaluation error: / by zero"); + assertThat(exception).hasCauseThat().hasMessageThat().contains("evaluation error"); + assertThat(exception).hasCauseThat().hasMessageThat().contains("/ by zero"); } } diff --git a/codelab/src/test/codelab/solutions/Exercise8Test.java b/codelab/src/test/codelab/solutions/Exercise8Test.java index a0d770a0..ac39340e 100644 --- a/codelab/src/test/codelab/solutions/Exercise8Test.java +++ b/codelab/src/test/codelab/solutions/Exercise8Test.java @@ -128,7 +128,7 @@ public void optimize_constantFold_evaluateError() throws Exception { .contains("evaluation error at :15: key 'referer' is not present in map."); assertThat(e2) .hasMessageThat() - .contains("evaluation error at :0: key 'referer' is not present in map."); + .contains("evaluation error: key 'referer' is not present in map."); } @Test diff --git a/extensions/src/test/java/dev/cel/extensions/CelListsExtensionsTest.java b/extensions/src/test/java/dev/cel/extensions/CelListsExtensionsTest.java index da947d87..537caea5 100644 --- a/extensions/src/test/java/dev/cel/extensions/CelListsExtensionsTest.java +++ b/extensions/src/test/java/dev/cel/extensions/CelListsExtensionsTest.java @@ -76,7 +76,7 @@ public void flatten_negativeDepth_throws() { assertThat(e) .hasMessageThat() - .contains("evaluation error: Function 'list_flatten_list_int' failed"); + .contains("evaluation error at :17: Function 'list_flatten_list_int' failed"); assertThat(e).hasCauseThat().hasMessageThat().isEqualTo("Level must be non-negative"); } diff --git a/runtime/src/main/java/dev/cel/runtime/CelEvaluationExceptionBuilder.java b/runtime/src/main/java/dev/cel/runtime/CelEvaluationExceptionBuilder.java index b02500c8..f09eaeb4 100644 --- a/runtime/src/main/java/dev/cel/runtime/CelEvaluationExceptionBuilder.java +++ b/runtime/src/main/java/dev/cel/runtime/CelEvaluationExceptionBuilder.java @@ -44,9 +44,12 @@ public CelEvaluationExceptionBuilder setErrorCode(CelErrorCode errorCode) { @CanIgnoreReturnValue public CelEvaluationExceptionBuilder setMetadata(Metadata metadata, long exprId) { - this.errorLocation = - SafeStringFormatter.format( - " at %s:%d", metadata.getLocation(), metadata.getPosition(exprId)); + if (metadata.hasPosition(exprId)) { + this.errorLocation = + SafeStringFormatter.format( + " at %s:%d", metadata.getLocation(), metadata.getPosition(exprId)); + } + return this; } diff --git a/runtime/src/main/java/dev/cel/runtime/DefaultInterpreter.java b/runtime/src/main/java/dev/cel/runtime/DefaultInterpreter.java index f10d62ad..2a4e58e2 100644 --- a/runtime/src/main/java/dev/cel/runtime/DefaultInterpreter.java +++ b/runtime/src/main/java/dev/cel/runtime/DefaultInterpreter.java @@ -212,7 +212,7 @@ private IntermediateResult evalInternal(ExecutionFrame frame, CelExpr expr) } catch (RuntimeException e) { throw CelEvaluationExceptionBuilder.newBuilder(e.getMessage()) .setCause(e) - // .setMetadata(metadata, expr.id()) TODO: Uncomment in the upcoming cl + .setMetadata(metadata, expr.id()) .build(); } } @@ -419,13 +419,13 @@ private IntermediateResult evalCall(ExecutionFrame frame, CelExpr expr, CelCall return IntermediateResult.create(attr, dispatchResult); } catch (CelRuntimeException ce) { throw CelEvaluationExceptionBuilder.newBuilder(ce) - // .setMetadata(metadata, expr.id()) // TODO: Uncomment in the upcoming cl + .setMetadata(metadata, expr.id()) .build(); } catch (RuntimeException e) { throw CelEvaluationExceptionBuilder.newBuilder( "Function '%s' failed with arg(s) '%s'", overload.getOverloadId(), Joiner.on(", ").join(argArray)) - // .setMetadata(metadata, expr.id()) // TODO: Uncomment in the upcoming cl + .setMetadata(metadata, expr.id()) .setCause(e) .build(); } @@ -456,7 +456,7 @@ private ResolvedOverload findOverloadOrThrow( .build()); } catch (CelRuntimeException e) { throw CelEvaluationExceptionBuilder.newBuilder(e) - // .setMetadata(metadata, expr.id()) // TODO: Uncomment in the upcoming cl + .setMetadata(metadata, expr.id()) .build(); } } diff --git a/runtime/src/main/java/dev/cel/runtime/DefaultMetadata.java b/runtime/src/main/java/dev/cel/runtime/DefaultMetadata.java index 43cce4e9..98aeb9c2 100644 --- a/runtime/src/main/java/dev/cel/runtime/DefaultMetadata.java +++ b/runtime/src/main/java/dev/cel/runtime/DefaultMetadata.java @@ -43,6 +43,11 @@ public String getLocation() { @Override public int getPosition(long exprId) { ImmutableMap positions = ast.getSource().getPositionsMap(); - return positions.containsKey(exprId) ? positions.get(exprId) : 0; + return positions.getOrDefault(exprId, 0); + } + + @Override + public boolean hasPosition(long exprId) { + return ast.getSource().getPositionsMap().containsKey(exprId); } } diff --git a/runtime/src/main/java/dev/cel/runtime/Metadata.java b/runtime/src/main/java/dev/cel/runtime/Metadata.java index 6bbda654..05d60768 100644 --- a/runtime/src/main/java/dev/cel/runtime/Metadata.java +++ b/runtime/src/main/java/dev/cel/runtime/Metadata.java @@ -33,4 +33,7 @@ public interface Metadata { * Returns the character position of the node in the source. This is a 0-based character offset. */ int getPosition(long exprId); + + /** Checks if a source position recorded for the provided expression id. */ + boolean hasPosition(long exprId); } diff --git a/runtime/src/test/java/dev/cel/runtime/CelEvaluationExceptionBuilderTest.java b/runtime/src/test/java/dev/cel/runtime/CelEvaluationExceptionBuilderTest.java index 5459a6e0..b3bc32e2 100644 --- a/runtime/src/test/java/dev/cel/runtime/CelEvaluationExceptionBuilderTest.java +++ b/runtime/src/test/java/dev/cel/runtime/CelEvaluationExceptionBuilderTest.java @@ -69,6 +69,11 @@ public String getLocation() { public int getPosition(long exprId) { return 10; } + + @Override + public boolean hasPosition(long exprId) { + return true; + } }, 0); diff --git a/runtime/src/test/resources/arithmInt64_error.baseline b/runtime/src/test/resources/arithmInt64_error.baseline index 4c3d44d4..05080171 100644 --- a/runtime/src/test/resources/arithmInt64_error.baseline +++ b/runtime/src/test/resources/arithmInt64_error.baseline @@ -1,41 +1,41 @@ Source: 9223372036854775807 + 1 =====> bindings: {} -error: evaluation error: long overflow +error: evaluation error at test_location:20: long overflow error_code: NUMERIC_OVERFLOW Source: -9223372036854775808 - 1 =====> bindings: {} -error: evaluation error: long overflow +error: evaluation error at test_location:21: long overflow error_code: NUMERIC_OVERFLOW Source: -(-9223372036854775808) =====> bindings: {} -error: evaluation error: long overflow +error: evaluation error at test_location:0: long overflow error_code: NUMERIC_OVERFLOW Source: 5000000000 * 5000000000 =====> bindings: {} -error: evaluation error: long overflow +error: evaluation error at test_location:11: long overflow error_code: NUMERIC_OVERFLOW Source: (-9223372036854775808)/-1 =====> bindings: {} -error: evaluation error: most negative number wraps +error: evaluation error at test_location:22: most negative number wraps error_code: NUMERIC_OVERFLOW Source: 1 / 0 =====> bindings: {} -error: evaluation error: / by zero +error: evaluation error at test_location:2: / by zero error_code: DIVIDE_BY_ZERO Source: 1 % 0 =====> bindings: {} -error: evaluation error: / by zero -error_code: DIVIDE_BY_ZERO +error: evaluation error at test_location:2: / by zero +error_code: DIVIDE_BY_ZERO \ No newline at end of file diff --git a/runtime/src/test/resources/arithmUInt64_error.baseline b/runtime/src/test/resources/arithmUInt64_error.baseline index d965a46e..062e2800 100644 --- a/runtime/src/test/resources/arithmUInt64_error.baseline +++ b/runtime/src/test/resources/arithmUInt64_error.baseline @@ -1,29 +1,29 @@ Source: 18446744073709551615u + 1u =====> bindings: {} -error: evaluation error: range overflow on unsigned addition +error: evaluation error at test_location:22: range overflow on unsigned addition error_code: NUMERIC_OVERFLOW Source: 0u - 1u =====> bindings: {} -error: evaluation error: unsigned subtraction underflow +error: evaluation error at test_location:3: unsigned subtraction underflow error_code: NUMERIC_OVERFLOW Source: 5000000000u * 5000000000u =====> bindings: {} -error: evaluation error: multiply out of unsigned integer range +error: evaluation error at test_location:12: multiply out of unsigned integer range error_code: NUMERIC_OVERFLOW Source: 1u / 0u =====> bindings: {} -error: evaluation error: / by zero +error: evaluation error at test_location:3: / by zero error_code: DIVIDE_BY_ZERO Source: 1u % 0u =====> bindings: {} -error: evaluation error: / by zero -error_code: DIVIDE_BY_ZERO +error: evaluation error at test_location:3: / by zero +error_code: DIVIDE_BY_ZERO \ No newline at end of file diff --git a/runtime/src/test/resources/boolConversions.baseline b/runtime/src/test/resources/boolConversions.baseline index 2bc832ba..46117866 100644 --- a/runtime/src/test/resources/boolConversions.baseline +++ b/runtime/src/test/resources/boolConversions.baseline @@ -11,16 +11,4 @@ result: true Source: bool('false') || bool('FALSE') || bool('False') || bool('f') || bool('0') =====> bindings: {} -result: false - -Source: bool('TrUe') -=====> -bindings: {} -error: evaluation error: Type conversion error from 'string' to 'bool': [TrUe] -error_code: BAD_FORMAT - -Source: bool('FaLsE') -=====> -bindings: {} -error: evaluation error: Type conversion error from 'string' to 'bool': [FaLsE] -error_code: BAD_FORMAT +result: false \ No newline at end of file diff --git a/runtime/src/test/resources/boolConversions_error.baseline b/runtime/src/test/resources/boolConversions_error.baseline new file mode 100644 index 00000000..a2c6a11d --- /dev/null +++ b/runtime/src/test/resources/boolConversions_error.baseline @@ -0,0 +1,11 @@ +Source: bool('TrUe') +=====> +bindings: {} +error: evaluation error at test_location:4: Type conversion error from 'string' to 'bool': [TrUe] +error_code: BAD_FORMAT + +Source: bool('FaLsE') +=====> +bindings: {} +error: evaluation error at test_location:4: Type conversion error from 'string' to 'bool': [FaLsE] +error_code: BAD_FORMAT \ No newline at end of file diff --git a/runtime/src/test/resources/booleans.baseline b/runtime/src/test/resources/booleans.baseline index 98bbfba9..6c0fb5a1 100644 --- a/runtime/src/test/resources/booleans.baseline +++ b/runtime/src/test/resources/booleans.baseline @@ -41,54 +41,6 @@ declare y { bindings: {y=0} result: true -Source: 1 / y == 1 || false -declare x { - value bool -} -declare y { - value int -} -=====> -bindings: {y=0} -error: evaluation error: / by zero -error_code: DIVIDE_BY_ZERO - -Source: false || 1 / y == 1 -declare x { - value bool -} -declare y { - value int -} -=====> -bindings: {y=0} -error: evaluation error: / by zero -error_code: DIVIDE_BY_ZERO - -Source: 1 / y == 1 && true -declare x { - value bool -} -declare y { - value int -} -=====> -bindings: {y=0} -error: evaluation error: / by zero -error_code: DIVIDE_BY_ZERO - -Source: true && 1 / y == 1 -declare x { - value bool -} -declare y { - value int -} -=====> -bindings: {y=0} -error: evaluation error: / by zero -error_code: DIVIDE_BY_ZERO - Source: 1 / y == 1 && false declare x { value bool @@ -274,5 +226,4 @@ declare y { } =====> bindings: {} -result: true - +result: true \ No newline at end of file diff --git a/runtime/src/test/resources/booleans_error.baseline b/runtime/src/test/resources/booleans_error.baseline new file mode 100644 index 00000000..e7ecf568 --- /dev/null +++ b/runtime/src/test/resources/booleans_error.baseline @@ -0,0 +1,35 @@ +Source: 1 / y == 1 || false +declare y { + value int +} +=====> +bindings: {y=0} +error: evaluation error at test_location:2: / by zero +error_code: DIVIDE_BY_ZERO + +Source: false || 1 / y == 1 +declare y { + value int +} +=====> +bindings: {y=0} +error: evaluation error at test_location:11: / by zero +error_code: DIVIDE_BY_ZERO + +Source: 1 / y == 1 && true +declare y { + value int +} +=====> +bindings: {y=0} +error: evaluation error at test_location:2: / by zero +error_code: DIVIDE_BY_ZERO + +Source: true && 1 / y == 1 +declare y { + value int +} +=====> +bindings: {y=0} +error: evaluation error at test_location:10: / by zero +error_code: DIVIDE_BY_ZERO \ No newline at end of file diff --git a/runtime/src/test/resources/doubleConversions.baseline b/runtime/src/test/resources/doubleConversions.baseline index 682e56ed..3c073ec3 100644 --- a/runtime/src/test/resources/doubleConversions.baseline +++ b/runtime/src/test/resources/doubleConversions.baseline @@ -13,12 +13,6 @@ Source: double(-1) bindings: {} result: -1.0 -Source: double('bad') -=====> -bindings: {} -error: evaluation error: For input string: "bad" -error_code: BAD_FORMAT - Source: double(1.5) =====> bindings: {} diff --git a/runtime/src/test/resources/doubleConversions_error.baseline b/runtime/src/test/resources/doubleConversions_error.baseline new file mode 100644 index 00000000..69f77ccf --- /dev/null +++ b/runtime/src/test/resources/doubleConversions_error.baseline @@ -0,0 +1,5 @@ +Source: double('bad') +=====> +bindings: {} +error: evaluation error at test_location:6: For input string: "bad" +error_code: BAD_FORMAT \ No newline at end of file diff --git a/runtime/src/test/resources/int64Conversions.baseline b/runtime/src/test/resources/int64Conversions.baseline index 6dcb11ce..0066c87a 100644 --- a/runtime/src/test/resources/int64Conversions.baseline +++ b/runtime/src/test/resources/int64Conversions.baseline @@ -8,19 +8,7 @@ Source: int(2.1) bindings: {} result: 2 -Source: int(18446744073709551615u) -=====> -bindings: {} -error: evaluation error: unsigned out of int range -error_code: NUMERIC_OVERFLOW - -Source: int(1e99) -=====> -bindings: {} -error: evaluation error: double is out of range for int -error_code: NUMERIC_OVERFLOW - Source: int(42u) =====> bindings: {} -result: 42 +result: 42 \ No newline at end of file diff --git a/runtime/src/test/resources/int64Conversions_error.baseline b/runtime/src/test/resources/int64Conversions_error.baseline new file mode 100644 index 00000000..2a4bda87 --- /dev/null +++ b/runtime/src/test/resources/int64Conversions_error.baseline @@ -0,0 +1,11 @@ +Source: int(18446744073709551615u) +=====> +bindings: {} +error: evaluation error at test_location:3: unsigned out of int range +error_code: NUMERIC_OVERFLOW + +Source: int(1e99) +=====> +bindings: {} +error: evaluation error at test_location:3: double is out of range for int +error_code: NUMERIC_OVERFLOW \ No newline at end of file diff --git a/runtime/src/test/resources/lists.baseline b/runtime/src/test/resources/lists.baseline index 865b27a0..03859450 100644 --- a/runtime/src/test/resources/lists.baseline +++ b/runtime/src/test/resources/lists.baseline @@ -94,19 +94,4 @@ declare list { } =====> bindings: {y=1, list=[1, 2, 3]} -result: true - -Source: list[3] -declare x { - value cel.expr.conformance.proto3.TestAllTypes -} -declare y { - value int -} -declare list { - value list(int) -} -=====> -bindings: {y=1, list=[1, 2, 3]} -error: evaluation error: Index out of bounds: 3 -error_code: INDEX_OUT_OF_BOUNDS +result: true \ No newline at end of file diff --git a/runtime/src/test/resources/lists_error.baseline b/runtime/src/test/resources/lists_error.baseline new file mode 100644 index 00000000..7fd6cedd --- /dev/null +++ b/runtime/src/test/resources/lists_error.baseline @@ -0,0 +1,11 @@ +Source: list[3] +declare y { + value int +} +declare list { + value list(int) +} +=====> +bindings: {y=1, list=[1, 2, 3]} +error: evaluation error at test_location:4: Index out of bounds: 3 +error_code: INDEX_OUT_OF_BOUNDS \ No newline at end of file diff --git a/runtime/src/test/resources/optional_errors.baseline b/runtime/src/test/resources/optional_errors.baseline index 819cc0c1..18b2846f 100644 --- a/runtime/src/test/resources/optional_errors.baseline +++ b/runtime/src/test/resources/optional_errors.baseline @@ -1,5 +1,5 @@ Source: optional.unwrap([dyn(1)]) =====> bindings: {} -error: evaluation error: Function 'optional_unwrap_list' failed with arg(s) '[1]' -error_code: INTERNAL_ERROR +error: evaluation error at test_location:15: Function 'optional_unwrap_list' failed with arg(s) '[1]' +error_code: INTERNAL_ERROR \ No newline at end of file diff --git a/runtime/src/test/resources/regexpMatches_error.baseline b/runtime/src/test/resources/regexpMatches_error.baseline index a9d3174e..38ff6403 100644 --- a/runtime/src/test/resources/regexpMatches_error.baseline +++ b/runtime/src/test/resources/regexpMatches_error.baseline @@ -1,11 +1,11 @@ Source: matches("alpha", "**") =====> bindings: {} -error: evaluation error: error parsing regexp: missing argument to repetition operator: `*` +error: evaluation error at test_location:7: error parsing regexp: missing argument to repetition operator: `*` error_code: INVALID_ARGUMENT Source: "alpha".matches("**") =====> bindings: {} -error: evaluation error: error parsing regexp: missing argument to repetition operator: `*` -error_code: INVALID_ARGUMENT +error: evaluation error at test_location:15: error parsing regexp: missing argument to repetition operator: `*` +error_code: INVALID_ARGUMENT \ No newline at end of file diff --git a/runtime/src/test/resources/timeConversions.baseline b/runtime/src/test/resources/timeConversions.baseline index cb1c4e88..68892ee2 100644 --- a/runtime/src/test/resources/timeConversions.baseline +++ b/runtime/src/test/resources/timeConversions.baseline @@ -46,15 +46,6 @@ result: seconds: 3723 nanos: 400000000 -Source: duration('inf') -declare t1 { - value google.protobuf.Timestamp -} -=====> -bindings: {} -error: evaluation error: invalid duration format -error_code: BAD_FORMAT - Source: duration(duration('15.0s')) declare t1 { value google.protobuf.Timestamp diff --git a/runtime/src/test/resources/timeConversions_error.baseline b/runtime/src/test/resources/timeConversions_error.baseline new file mode 100644 index 00000000..3b331a3d --- /dev/null +++ b/runtime/src/test/resources/timeConversions_error.baseline @@ -0,0 +1,5 @@ +Source: duration('inf') +=====> +bindings: {} +error: evaluation error at test_location:8: invalid duration format +error_code: BAD_FORMAT \ No newline at end of file diff --git a/runtime/src/test/resources/uint64Conversions.baseline b/runtime/src/test/resources/uint64Conversions.baseline index fac9bfc5..9f858f47 100644 --- a/runtime/src/test/resources/uint64Conversions.baseline +++ b/runtime/src/test/resources/uint64Conversions.baseline @@ -8,34 +8,16 @@ Source: uint(2.1) bindings: {} result: 2 -Source: uint(-1) -=====> -bindings: {} -error: evaluation error: int out of uint range -error_code: NUMERIC_OVERFLOW - Source: uint(1e19) =====> bindings: {} result: 10000000000000000000 -Source: uint(6.022e23) -=====> -bindings: {} -error: evaluation error: double out of uint range -error_code: NUMERIC_OVERFLOW - Source: uint(42) =====> bindings: {} result: 42 -Source: uint('f1') -=====> -bindings: {} -error: evaluation error: f1 -error_code: BAD_FORMAT - Source: uint(1u) =====> bindings: {} @@ -44,4 +26,4 @@ result: 1 Source: uint(dyn(1u)) =====> bindings: {} -result: 1 +result: 1 \ No newline at end of file diff --git a/runtime/src/test/resources/uint64Conversions_error.baseline b/runtime/src/test/resources/uint64Conversions_error.baseline new file mode 100644 index 00000000..0a47ccf8 --- /dev/null +++ b/runtime/src/test/resources/uint64Conversions_error.baseline @@ -0,0 +1,17 @@ +Source: uint(-1) +=====> +bindings: {} +error: evaluation error at test_location:4: int out of uint range +error_code: NUMERIC_OVERFLOW + +Source: uint(6.022e23) +=====> +bindings: {} +error: evaluation error at test_location:4: double out of uint range +error_code: NUMERIC_OVERFLOW + +Source: uint('f1') +=====> +bindings: {} +error: evaluation error at test_location:4: f1 +error_code: BAD_FORMAT \ No newline at end of file diff --git a/runtime/src/test/resources/unknownResultSet.baseline b/runtime/src/test/resources/unknownResultSet.baseline index 65ec7daa..8c718376 100644 --- a/runtime/src/test/resources/unknownResultSet.baseline +++ b/runtime/src/test/resources/unknownResultSet.baseline @@ -60,7 +60,7 @@ declare x { } =====> bindings: {} -error: evaluation error: Failed to parse timestamp: invalid timestamp "bad timestamp string" +error: evaluation error at test_location:31: Failed to parse timestamp: invalid timestamp "bad timestamp string" error_code: BAD_FORMAT Source: x.single_int32 == 1 || x.single_string == "test" @@ -125,7 +125,7 @@ declare x { } =====> bindings: {} -error: evaluation error: Failed to parse timestamp: invalid timestamp "bad timestamp string" +error: evaluation error at test_location:31: Failed to parse timestamp: invalid timestamp "bad timestamp string" error_code: BAD_FORMAT Source: x.single_int32.f(1) @@ -427,5 +427,5 @@ declare f { } =====> bindings: {} -error: evaluation error: / by zero -error_code: DIVIDE_BY_ZERO +error: evaluation error at test_location:7: / by zero +error_code: DIVIDE_BY_ZERO \ No newline at end of file diff --git a/testing/src/main/java/dev/cel/testing/BaseInterpreterTest.java b/testing/src/main/java/dev/cel/testing/BaseInterpreterTest.java index a14d359a..b7f52476 100644 --- a/testing/src/main/java/dev/cel/testing/BaseInterpreterTest.java +++ b/testing/src/main/java/dev/cel/testing/BaseInterpreterTest.java @@ -146,7 +146,7 @@ protected void prepareCompiler(CelTypeProvider typeProvider) { private CelAbstractSyntaxTree compileTestCase() { CelAbstractSyntaxTree ast = prepareTest(TEST_FILE_DESCRIPTORS); if (ast == null) { - return ast; + return null; } assertAstRoundTrip(ast); @@ -179,6 +179,11 @@ private Object runTest(Map input, CelLateFunctionBindings lateFunctio private Object runTestInternal( Object input, Optional lateFunctionBindings) { CelAbstractSyntaxTree ast = compileTestCase(); + if (ast == null) { + // Usually indicates test was not setup correctly + println("Source compilation failed"); + return null; + } printBinding(input); Object result = null; try { @@ -411,18 +416,6 @@ public void booleans() { source = "1 / y == 1 || true"; runTest(ImmutableMap.of("y", 0L)); - source = "1 / y == 1 || false"; - runTest(ImmutableMap.of("y", 0L)); - - source = "false || 1 / y == 1"; - runTest(ImmutableMap.of("y", 0L)); - - source = "1 / y == 1 && true"; - runTest(ImmutableMap.of("y", 0L)); - - source = "true && 1 / y == 1"; - runTest(ImmutableMap.of("y", 0L)); - source = "1 / y == 1 && false"; runTest(ImmutableMap.of("y", 0L)); @@ -475,6 +468,23 @@ public void booleans() { runTest(); } + @Test + public void booleans_error() { + declareVariable("y", CelProtoTypes.INT64); + + source = "1 / y == 1 || false"; + runTest(ImmutableMap.of("y", 0L)); + + source = "false || 1 / y == 1"; + runTest(ImmutableMap.of("y", 0L)); + + source = "1 / y == 1 && true"; + runTest(ImmutableMap.of("y", 0L)); + + source = "true && 1 / y == 1"; + runTest(ImmutableMap.of("y", 0L)); + } + @Test public void strings() throws Exception { source = "'a' < 'b' && 'a' <= 'b' && 'b' > 'a' && 'a' >= 'a' && 'a' == 'a' && 'a' != 'b'"; @@ -745,6 +755,12 @@ public void lists() throws Exception { source = "y in list"; runTest(ImmutableMap.of("y", 1L, "list", ImmutableList.of(1L, 2L, 3L))); + } + + @Test + public void lists_error() { + declareVariable("y", CelProtoTypes.INT64); + declareVariable("list", CelProtoTypes.createList(CelProtoTypes.INT64)); source = "list[3]"; runTest(ImmutableMap.of("y", 1L, "list", ImmutableList.of(1L, 2L, 3L))); @@ -1348,10 +1364,6 @@ public void timeConversions() { source = "duration(\"1h2m3.4s\")"; runTest(); - // Not supported. - source = "duration('inf')"; - runTest(); - source = "duration(duration('15.0s'))"; // Identity runTest(); @@ -1359,6 +1371,12 @@ public void timeConversions() { runTest(); } + @Test + public void timeConversions_error() { + source = "duration('inf')"; + runTest(); + } + @Test public void sizeTests() { container = Type.getDescriptor().getFile().getPackage(); @@ -1549,13 +1567,16 @@ public void int64Conversions() { source = "int(2.1)"; // double converts to 2 runTest(); - source = "int(18446744073709551615u)"; // 2^64-1 should error + source = "int(42u)"; // converts to 42 runTest(); + } - source = "int(1e99)"; // out of range should error + @Test + public void int64Conversions_error() { + source = "int(18446744073709551615u)"; // 2^64-1 should error runTest(); - source = "int(42u)"; // converts to 42 + source = "int(1e99)"; // out of range should error runTest(); } @@ -1573,25 +1594,28 @@ public void uint64Conversions() { source = "uint(2.1)"; // double converts to 2u runTest(); - source = "uint(-1)"; // should error + source = "uint(1e19)"; // valid uint but outside of int range runTest(); - source = "uint(1e19)"; // valid uint but outside of int range + source = "uint(42)"; // int converts to 42u runTest(); - source = "uint(6.022e23)"; // outside uint range + source = "uint(1u)"; // identity runTest(); - source = "uint(42)"; // int converts to 42u + source = "uint(dyn(1u))"; // identity, check dynamic dispatch runTest(); + } - source = "uint('f1')"; // should error + @Test + public void uint64Conversions_error() { + source = "uint(-1)"; // should error runTest(); - source = "uint(1u)"; // identity + source = "uint(6.022e23)"; // outside uint range runTest(); - source = "uint(dyn(1u))"; // identity, check dynamic dispatch + source = "uint('f1')"; // should error runTest(); } @@ -1606,10 +1630,13 @@ public void doubleConversions() { source = "double(-1)"; // int converts to -1.0 runTest(); - source = "double('bad')"; + source = "double(1.5)"; // Identity runTest(); + } - source = "double(1.5)"; // Identity + @Test + public void doubleConversions_error() { + source = "double('bad')"; runTest(); } @@ -1664,12 +1691,15 @@ public void boolConversions() { source = "bool('false') || bool('FALSE') || bool('False') || bool('f') || bool('0')"; runTest(); // result is false + } + @Test + public void boolConversions_error() { source = "bool('TrUe')"; - runTest(); // exception + runTest(); source = "bool('FaLsE')"; - runTest(); // exception + runTest(); } @Test diff --git a/validator/src/test/java/dev/cel/validator/validators/DurationLiteralValidatorTest.java b/validator/src/test/java/dev/cel/validator/validators/DurationLiteralValidatorTest.java index 3fff1c2b..ecc2aaa5 100644 --- a/validator/src/test/java/dev/cel/validator/validators/DurationLiteralValidatorTest.java +++ b/validator/src/test/java/dev/cel/validator/validators/DurationLiteralValidatorTest.java @@ -89,7 +89,9 @@ public void duration_withVariable_noOp() throws Exception { assertThrows( CelEvaluationException.class, () -> CEL.createProgram(ast).eval(ImmutableMap.of("str_var", "bad"))); - assertThat(e).hasMessageThat().contains("evaluation error: invalid duration format"); + assertThat(e) + .hasMessageThat() + .contains("evaluation error at :8: invalid duration format"); } @Test @@ -124,7 +126,9 @@ public void duration_withFunction_noOp() throws Exception { // However, the same AST fails on evaluation when the function dispatch fails. assertThat(e) .hasMessageThat() - .contains("evaluation error: Function 'testFuncOverloadId' failed with arg(s) 'bad'"); + .contains( + "evaluation error at :17: Function 'testFuncOverloadId' failed with arg(s)" + + " 'bad'"); } @Test diff --git a/validator/src/test/java/dev/cel/validator/validators/RegexLiteralValidatorTest.java b/validator/src/test/java/dev/cel/validator/validators/RegexLiteralValidatorTest.java index f54e487d..32fa0502 100644 --- a/validator/src/test/java/dev/cel/validator/validators/RegexLiteralValidatorTest.java +++ b/validator/src/test/java/dev/cel/validator/validators/RegexLiteralValidatorTest.java @@ -97,7 +97,8 @@ public void regex_globalWithVariable_noOp() throws Exception { assertThat(e) .hasMessageThat() .contains( - "evaluation error: error parsing regexp: missing argument to repetition operator: `*`"); + "evaluation error at :7: error parsing regexp: missing argument to repetition" + + " operator: `*`"); } @Test @@ -118,7 +119,8 @@ public void regex_receiverWithVariable_noOp() throws Exception { assertThat(e) .hasMessageThat() .contains( - "evaluation error: error parsing regexp: missing argument to repetition operator: `*`"); + "evaluation error at :14: error parsing regexp: missing argument to repetition" + + " operator: `*`"); } @Test @@ -144,7 +146,8 @@ public void regex_globalWithFunction_noOp() throws Exception { assertThat(e) .hasMessageThat() .contains( - "evaluation error: error parsing regexp: missing argument to repetition operator: `*`"); + "evaluation error at :7: error parsing regexp: missing argument to repetition" + + " operator: `*`"); } @Test @@ -170,7 +173,8 @@ public void regex_receiverWithFunction_noOp() throws Exception { assertThat(e) .hasMessageThat() .contains( - "evaluation error: error parsing regexp: missing argument to repetition operator: `*`"); + "evaluation error at :14: error parsing regexp: missing argument to repetition" + + " operator: `*`"); } @Test diff --git a/validator/src/test/java/dev/cel/validator/validators/TimestampLiteralValidatorTest.java b/validator/src/test/java/dev/cel/validator/validators/TimestampLiteralValidatorTest.java index 0477dace..e6208518 100644 --- a/validator/src/test/java/dev/cel/validator/validators/TimestampLiteralValidatorTest.java +++ b/validator/src/test/java/dev/cel/validator/validators/TimestampLiteralValidatorTest.java @@ -100,7 +100,8 @@ public void timestamp_withVariable_noOp() throws Exception { () -> CEL.createProgram(ast).eval(ImmutableMap.of("str_var", "bad"))); assertThat(e) .hasMessageThat() - .contains("evaluation error: Failed to parse timestamp: invalid timestamp \"bad\""); + .contains( + "evaluation error at :9: Failed to parse timestamp: invalid timestamp \"bad\""); } @Test @@ -136,7 +137,9 @@ public void timestamp_withFunction_noOp() throws Exception { // However, the same AST fails on evaluation when the function dispatch fails. assertThat(e) .hasMessageThat() - .contains("evaluation error: Function 'testFuncOverloadId' failed with arg(s) 'bad'"); + .contains( + "evaluation error at :18: Function 'testFuncOverloadId' failed with arg(s)" + + " 'bad'"); } @Test