Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tests that fail before the scheduler are not handled properly #1253

Closed
ktbarrett opened this issue Dec 22, 2019 · 6 comments · Fixed by #1262
Closed

Tests that fail before the scheduler are not handled properly #1253

ktbarrett opened this issue Dec 22, 2019 · 6 comments · Fixed by #1262
Labels
category:core (DEPRECATED) Issues in the core of cocotb (scheduler, error reporting, etc.) type:bug a bug in existing functionality

Comments

@ktbarrett
Copy link
Member

For example:

@cocotb.test(expect_fail=True)
def test_not_a_coroutine(dut):
"""Example of a failing to use the yield keyword in a test"""
dut._log.warning("This test will fail because we don't yield anything")

This is considered "N/A" in the regression results banner and passes whether expect_fail or expect_error are set to True or False. A traceback is printed, but it's not apparent there was any failure if you are running a number of tests, like in test_cocotb.

The regression manager nor the cocotb.test decorator make any attempt to handle potential issues.

self._queue.append(_test(self._dut))

cocotb/cocotb/decorators.py

Lines 477 to 478 in 92a032a

def __call__(self, *args, **kwargs):
return RunningTest(self._func(*args, **kwargs), self)

I put some time into refactoring initialize and handle_result in the regression manager, but felt like I more or less had to copy-paste to victory, which I didn't like. I think a larger refactor is in order.

@ktbarrett
Copy link
Member Author

Note, this isn't an issue if the test coroutine is an async function. When async functions are called, nothing executes immediately, so they cannot fail until they get to the scheduler.

@eric-wieser
Copy link
Member

Pretty sure we could detect this with insoect.isgeneratorfunction or similar

@ktbarrett
Copy link
Member Author

ktbarrett commented Dec 22, 2019

I just didn't find a clean way of modifying handle_result to call it from initialize in the error case, handle_result expects a RunningTest object.

@eric-wieser
Copy link
Member

PR appreciated, this has been annoying me for a while when writing and screwing up new tests.

@ktbarrett
Copy link
Member Author

ktbarrett commented Dec 24, 2019

I'm actually wondering if instead of modifying the regression manager I should modify the decorators so that __call__ does not immediately run the coroutine's body to the first yield. async coroutine functions have that deferred execution behavior, it doesn't make much sense for async and non-async coroutines to have different behavior.

If that's the case, this will wait until #1255 or #1259 are merged.

EDIT: I didn't quite understand what was going on, but now I do. Cocotb coroutines are not immediately started just like async coroutines. The issue is with errors that cause coroutine initialization to fail like NameError, not necessarily any exception before the first yield.

ktbarrett added a commit to ktbarrett/cocotb that referenced this issue Dec 24, 2019
Fixes cocotb#1253. Code is cleaned up slightly. More PEP8 changes. More
comments.
ktbarrett added a commit to ktbarrett/cocotb that referenced this issue Dec 24, 2019
Fixes cocotb#1253. Code is cleaned up slightly. More PEP8 changes. More
comments.
ktbarrett added a commit to ktbarrett/cocotb that referenced this issue Dec 24, 2019
Fixes cocotb#1253. Code is cleaned up slightly. More PEP8 changes. More
comments.
@eric-wieser
Copy link
Member

The issue is with errors that cause coroutine initialization to fail like NameError, not necessarily any exception before the first yield.

I suspect the issue is actually errors during coroutine initialization, such as:

@cocotb.coroutine
def not_a_coroutine(dut):
    raise RuntimeError()

or

@cocotb.coroutine
def returns_a_coroutine(dut):
    return the_coroutine_with_a_typo()

ktbarrett added a commit to ktbarrett/cocotb that referenced this issue Dec 25, 2019
Fixes cocotb#1253. Code is cleaned up slightly. More PEP8 changes. More
comments.
ktbarrett added a commit to ktbarrett/cocotb that referenced this issue Dec 26, 2019
Fixes cocotb#1253. Code is cleaned up slightly. More PEP8 changes. More
comments.
ktbarrett added a commit to ktbarrett/cocotb that referenced this issue Dec 26, 2019
Fixes cocotb#1253. Code is cleaned up slightly. More PEP8 changes. More
comments.
ktbarrett added a commit to ktbarrett/cocotb that referenced this issue Dec 26, 2019
Fixes cocotb#1253. Code is cleaned up slightly. More PEP8 changes. More
comments.
ktbarrett added a commit to ktbarrett/cocotb that referenced this issue Dec 26, 2019
Fixes cocotb#1253. Code is cleaned up slightly. More PEP8 changes. More
comments.
ktbarrett added a commit to ktbarrett/cocotb that referenced this issue Dec 28, 2019
Fixes cocotb#1253. Code is cleaned up slightly. More PEP8 changes. More
comments.
ktbarrett added a commit to ktbarrett/cocotb that referenced this issue Dec 28, 2019
Fixes cocotb#1253. Code is cleaned up slightly. More PEP8 changes. More
comments.
ktbarrett added a commit to ktbarrett/cocotb that referenced this issue Dec 28, 2019
Fixes cocotb#1253. Code is cleaned up slightly. More PEP8 changes. More
comments.
ktbarrett added a commit to ktbarrett/cocotb that referenced this issue Jan 2, 2020
Fixes cocotb#1253. Code is cleaned up slightly. More PEP8 changes. More
comments.
ktbarrett added a commit to ktbarrett/cocotb that referenced this issue Jan 3, 2020
Fixes cocotb#1253. Code is cleaned up slightly. More PEP8 changes. More
comments.
ktbarrett added a commit to ktbarrett/cocotb that referenced this issue Jan 3, 2020
Fixes cocotb#1253. Code is cleaned up slightly. More PEP8 changes. More
comments.
ktbarrett added a commit to ktbarrett/cocotb that referenced this issue Jan 10, 2020
Fixes cocotb#1253. Code is cleaned up slightly. More PEP8 changes. More
comments.
ktbarrett added a commit to ktbarrett/cocotb that referenced this issue Jan 23, 2020
Fixes cocotb#1253. Code is cleaned up slightly. More PEP8 changes. More
comments.
ktbarrett added a commit to ktbarrett/cocotb that referenced this issue Jan 25, 2020
Fixes cocotb#1253. Code is cleaned up slightly. More PEP8 changes. More
comments.
ktbarrett added a commit to ktbarrett/cocotb that referenced this issue Jan 28, 2020
Fixes cocotb#1253. Code is cleaned up slightly. More PEP8 changes. More
comments.
ktbarrett added a commit to ktbarrett/cocotb that referenced this issue Feb 2, 2020
Fixes cocotb#1253. Code is cleaned up slightly. More PEP8 changes. More
comments.
ktbarrett added a commit to ktbarrett/cocotb that referenced this issue Feb 6, 2020
There are two places in the RM that initialize coroutines, each had
slightly different code, so this was factored out into a method
init_test.

init_test can fail, and so can a test at any point in it's run
(previously handled by handle_result), so scoring a test as a pass or a
fail depending upon the result and expected behavior is factored out
into a method score_test.

Now results are handled during initialization, which fixes cocotb#1253.
eric-wieser pushed a commit that referenced this issue Feb 7, 2020
There are two places in the RM that initialize coroutines, each had
slightly different code, so this was factored out into a method
init_test.

init_test can fail, and so can a test at any point in it's run
(previously handled by handle_result), so scoring a test as a pass or a
fail depending upon the result and expected behavior is factored out
into a method score_test.

Now results are handled during initialization, which fixes #1253.
@eric-wieser eric-wieser added category:core (DEPRECATED) Issues in the core of cocotb (scheduler, error reporting, etc.) type:bug a bug in existing functionality labels Feb 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:core (DEPRECATED) Issues in the core of cocotb (scheduler, error reporting, etc.) type:bug a bug in existing functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants