Add conformance tests for trap-expr #1085
Conversation
KavinduZoysa
requested review from
sameerajayasoma,
hasithaa,
rdhananjaya and
MaryamZi
as code owners
| function foo() returns int { | ||
| return 0; | ||
| } | ||
|
|
||
| function errorFunction() { | ||
| int _ = trap 2 / foo(); // @error incompatible types: expected 'int', found '(int|error)' | ||
| error e = trap 2 / foo(); // @error incompatible types: expected 'error', found 'int' |
There was a problem hiding this comment.
Isn't the test overly complicated? We don't need a function call. Even a variable reference would suffice.
| function foo() returns int { | ||
| return 0; | ||
| } | ||
|
|
||
| function errorFunction() { | ||
| int _ = trap 2 / foo(); // @error incompatible types: expected 'int', found '(int|error)' | ||
| error e = trap 2 / foo(); // @error incompatible types: expected 'error', found 'int' |
There was a problem hiding this comment.
Created an issue and moved this test to failing tests
| int|error t1 = trap trap 2 / foo(); | ||
| io:println(t1); // @output error("{ballerina}DivisionByZero",message=" / by zero") | ||
| } | ||
|
|
There was a problem hiding this comment.
Do you have tests for when the expression doesn't panic?
There was a problem hiding this comment.
Added few tests at the bottom
There was a problem hiding this comment.
Is it the test immediately after this? Can we improve the descriptions to specifically mention that they don't panic?
KavinduZoysa
force-pushed
the
trap-expr-conformance-tests
branch
from
cf23ec1 to
8d3942d
Compare
| @@ -0,0 +1,351 @@ | |||
| Test-Case: output | |||
| Description: Test evaluation of expression resulting in value v, | |||
There was a problem hiding this comment.
Do we need this? It's not clear what v is. I would say
| Description: Test evaluation of expression resulting in value v, | |
| Description: Test the result of the trap expression being `e`, when evaluation of the expression completes abruptly with panic with associated value `e`. |
There was a problem hiding this comment.
Please recheck all descriptions. We need to make the concise, but self-explanatory and clear.
| } | ||
|
|
||
| Test-Case: output | ||
| Description: Test trap expression when the expression is a type-cast-expr. |
There was a problem hiding this comment.
| Description: Test trap expression when the expression is a type-cast-expr. | |
| Description: Test trap expression when the expression is a type cast expression. |
Please update wherever applicable.
| int|error t1 = trap trap 2 / foo(); | ||
| io:println(t1); // @output error("{ballerina}DivisionByZero",message=" / by zero") | ||
| } | ||
|
|
There was a problem hiding this comment.
Is it the test immediately after this? Can we improve the descriptions to specifically mention that they don't panic?
KavinduZoysa
force-pushed
the
trap-expr-conformance-tests
branch
from
8d3942d to
fb67a16
Compare
| } | ||
|
|
||
| Test-Case: output | ||
| Description: Test trap expression when the expression is a numeric-literal and it does not panic. |
There was a problem hiding this comment.
When does a numeric literal panic?
There was a problem hiding this comment.
Thoughts on this?
Let's also remove the productions from the descriptions everywhere.
numeric-literal -> numeric literal
There was a problem hiding this comment.
Fixed in 5303f62
| } | ||
|
|
||
| Test-Case: output | ||
| Description: Test trap expression when the expression is a byte value and it does not panic. |
There was a problem hiding this comment.
Can we add some tests with function calls here instead of literals and variable reference expressions that we know will never panic?
There was a problem hiding this comment.
@MaryamZi, do you mean here to add function calls that causes panic or add functions that return literal?
There was a problem hiding this comment.
Like a function call, which at compile-time we don't know if it will panic.
function init() {
int|error a = trap fn("1");
io:println(a);
}
function fn(string str) returns int {
return checkpanic int:fromString(str);
}
KavinduZoysa
force-pushed
the
trap-expr-conformance-tests
branch
from
530a9c8 to
5303f62
Compare
| } | ||
|
|
||
| Test-Case: output | ||
| Description: Test trap expression when the expression is a function call which returns subtype of int or panic. |
There was a problem hiding this comment.
| Description: Test trap expression when the expression is a function call which returns subtype of int or panic. | |
| Description: Test trap expression when the expression is a function call which returns a subtype of int or panics. |
There was a problem hiding this comment.
Please fix wherever applicable.
| } | ||
|
|
||
| Test-Case: output | ||
| Description: Test trap expression when the expression is a query expression |
There was a problem hiding this comment.
| Description: Test trap expression when the expression is a query expression | |
| Description: Test trap expression when the expression is a query expression. |
|
|
||
| function init() { | ||
| int zero = 0; | ||
| int|error e1 = trap (check 2 / zero); |
There was a problem hiding this comment.
I think this is a bug. trap captures a panic. check should return the error from the function. Can you recheck in the spec and create a spec issue if required or a lang issue?
| int[] i = []; | ||
| return i[3]; |
There was a problem hiding this comment.
Missing member access related labels in Labels.
|
|
||
| function init() { | ||
| Obj obj = new(); | ||
| error? t = trap obj.bar(); |
There was a problem hiding this comment.
This doesn't match the description.
Test trap expression when the expression is a new expression.
| panic error("msg"); | ||
| } | ||
| }; | ||
| error? e = trap obj.bar(); |
There was a problem hiding this comment.
Also doesn't match the description.
Purpose
Fixes ballerina-platform/ballerina-lang#35814