I was writing some unit tests for tasks and ran across this issue. I thought it was worth sharing.

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.

you can assess the underlying wrapped function with

f.task_function

Ah, great! Thanks, @Niels Bantilan

def test_f_raises():
    with pytest.raises(ValueError):
        _f = f.task_function
        _f(-1)

works!

cc @Eduardo Apolinario (eapolinario)

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?

What tests broke @Charlie Moriarty?

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:

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:

def test_f_raises():
    with pytest.raises(ValueError):
        _f = f.task_function
        _f(-1)

@Len Strnad @Charlie Moriarty would one of you mind creating an issue for this? https://github.com/flyteorg/flyte/issues/new?assignees=&labels=bug%2Cuntriaged&projects=&template=bug_report.yaml&title=%5BBUG%5D+

Done! https://github.com/flyteorg/flyte/issues/5433

Just want to make sure we got to the bottom of this. Please, read this comment on the issue.

Thanks @Eduardo Apolinario (eapolinario)! 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 👍