I was writing some unit tests for tasks and ran ac...
# flyte-support
b
I was writing some unit tests for tasks and ran across this issue. I thought it was worth sharing.
Copy code
from flytekit import task

@task
def f(x: int) -> int:
    if x > 0:
        return x + 2
    else:
        raise ValueError("x must be positive")
    

def test_f_raises():
    with pytest.raises(ValueError):
        f(-1)
Will fail since flyte doesn’t seem to surface the ValueError, but instead shows a SystemExit. Any tips to bypass this type of behavior? I can easily abstract the logic out of
f
and test that python function independently from task
f
, but for more experimental workflows, we tend to avoid these decoupling abstractions.
βœ… 1
b
you can assess the underlying wrapped function with
f.task_function
although I feel like the correct behavior here would be to flytekit raise the proper error in the context of a local task execution
b
Ah, great! Thanks, @broad-monitor-993
Copy code
def test_f_raises():
    with pytest.raises(ValueError):
        _f = f.task_function
        _f(-1)
works!
That would be nice, but this is a fine workaround as long as one knows about the
.task_function
property
t
cc @high-accountant-32689
another bit to clean up. the error handling in flytekit became too complex as we tried to make it able to distinguish between user errors and flytekit/flyte system related errors
a
We also experienced this issue when upgrading flytekit from 1.11.0 -> 1.12.0 and it broke many of our unit tests πŸ˜… Are there any plans to document migration guides with future releases?
b
What tests broke @abundant-judge-84756?
a
Tests similar to the example in the initial post - we have tests for the error behaviour of multiple flyte task functions,
f
, which previously passed using:
Copy code
def test_f_raises():
    with pytest.raises(ValueError):
        f(-1)
Now in order to pass in
1.12.0
these have all been converted to:
Copy code
def test_f_raises():
    with pytest.raises(ValueError):
        _f = f.task_function
        _f(-1)
b
πŸ‘ 1
a
h
Just want to make sure we got to the bottom of this. Please, read this comment on the issue.
a
Thanks @high-accountant-32689! All good on our end, we're not blocked now that we've added the extra code to fix the tests - but definitely sounds good to revisit the change in future so that the additional function dives or env var aren't needed πŸ‘
πŸ‘ 1