`assertThrows` in sdkTests should be used in wing `test` code block

[fynnfluegge]

Feature Spec

examples/tests/sdk_tests are using try ... catch in wing test { } code block. Should be assertThrows instead, since assert is only called on exception catch and ignored otherwise.

let PARSE_ERROR = "unable to parse number 123 as a string";

assert(str.fromJson(Json "Hello") == "Hello");
assertThrows(PARSE_ERROR, () => {
  str.fromJson(Json 123); 
});

test "fromJson" {
  assert(str.fromJson(Json "World") == "World");
  try { str.fromJson(Json 123); } catch s { assert(s == PARSE_ERROR); }  <-- assert will only be visited on exception
}

Use Cases

Tests ensure that the exception is thrown with the correct error message.

Implementation Notes

Using predefined assertThrows method fails with:

error: Cannot call into preflight phase while inflight

Component

SDK

Community Notes

• Please vote by adding a 👍 reaction to the issue to help us prioritize.
• If you are interested to work on this issue, please leave a comment.

[ekeren]

@fynnfluegge,

I usually don't care about the error message that is thrown:

// So I would us:
assertThrows(() => {
  str.fromJson(Json 123); 
});

// In your example, you can use:
assertThrows(() => {
  str.fromJson(Json 123); 
}, expectedErrorMessage: PARSE_ERROR);

The last argument in this case is AssertThrowOptions struct that has expectedErrorMessage: str? field and if it is missing it shouldn't assert that message at all

[fynnfluegge]

// So I would us:
assertThrows(() => {
  str.fromJson(Json 123); 
});

// In your example, you can use:
assertThrows(() => {
  str.fromJson(Json 123); 
}, expectedErrorMessage: PARSE_ERROR);

The last argument in this case is AssertThrowOptions struct that has expectedErrorMessage: str? field and if it is missing it shouldn't assert that message at all

Alright but how to use this inisde test "fromJson" { }? This try ... catch would pass if there is no exception

[ekeren]

Alright but how to use this inisde test "fromJson" { }? This try ... catch would pass if there is no exception I think that because It excepts an exception it should fail if exception is not thrown:

a possible implementation of this is

inflight assertThrows (code: inflight ():void, options: AssertThrowOptions) {
   try {
       code(); 
       assert(false); // fail in case exception is not thrown
   } catch  {
     // verify exception message
   } 
}

[fynnfluegge]

inflight assertThrows (code: inflight ():void, options: AssertThrowOptions) {
   try {
       code(); 
       assert(false); // fail in case exception is not thrown
   } catch  {
     // verify exception message
   } 
}

Nice 👍 So, would make sense to adapt this to any test in the sdkTests and remove this try .. catch right?

[ekeren]

Yes, makes sense to me.

[github-actions[bot]]

Hi,

This issue hasn't seen activity in 60 days. Therefore, we are marking this issue as stale for now. It will be closed after 7 days. Feel free to re-open this issue when there's an update or relevant information to be added. Thanks!

[github-actions[bot]]

Hi,

This issue hasn't seen activity in 60 days. Therefore, we are marking this issue as stale for now. It will be closed after 7 days. Feel free to re-open this issue when there's an update or relevant information to be added. Thanks!

[staycoolcall911]

@tsuf239, @Chriscbr, @skorfmann - would it make sense to add expect.throws(e) to our builtin expect module and use that instead of duplicating the assertThrows function code and the older try .. catch anti-pattern across the SDK spec tests?

[tsuf239]

@tsuf239, @Chriscbr, @skorfmann - would it make sense to add expect.throws(e) to our builtin expect module and use that instead of duplicating the assertThrows function code and the older try .. catch anti-pattern across the SDK spec tests?

yes! but it would have to be expect(() => statement).throws(e) or expect.throws(() => statement, e)

[github-actions[bot]]

Hi,

This issue hasn't seen activity in 90 days. Therefore, we are marking this issue as stale for now. It will be closed after 7 days. Feel free to re-open this issue when there's an update or relevant information to be added. Thanks!

[github-actions[bot]]

Hi,

This issue hasn't seen activity in 90 days. Therefore, we are marking this issue as stale for now. It will be closed after 7 days. Feel free to re-open this issue when there's an update or relevant information to be added. Thanks!