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

Prevent unity from bailing on failed tests #616

Open
lmapii opened this issue Jun 14, 2022 · 13 comments
Open

Prevent unity from bailing on failed tests #616

lmapii opened this issue Jun 14, 2022 · 13 comments

Comments

@lmapii
Copy link

lmapii commented Jun 14, 2022

I'm using unity on target and it works beautifully. However, when I have a test that fails, then all the other tests are skipped without any output due to the UNITY_FAIL_AND_BAIL macro in the assertion functions (leading to TEST_ABORT).

So far the only way to prevent this is to define UNITY_EXCLUDE_SETJMP_H in my unity_conf.h file, which might have some impact on mocking. Is there no other way to do this? I've never seen this behaviour when using ceedling, there the test execution continues on failures.

@mvandervoord
Copy link
Member

Hi.

Unity aborts the current test on a failure (when used by itself or with Ceedling). It's the "current test" portion that is important here. In either case, it should move on to the next test.

My suspicion is that something about your structure on your target tests isn't quite right. Maybe all your tests are getting called from a single RUN_TEST() call? Maybe you don't have RUN_TEST() wrappers around your tests at all? Depending on your target, people also can run into situations where their tests run into memory problems, and Unity has no way to recover from it and so execution just stops.

Are you using a generated test runner or are you creating your own? If the latter, it'd be good to see an example of what that looks like.

Are you using the default RUN_TEST macro, or have you created your own? If the latter, it'd be good to see that as well. :)

@lmapii
Copy link
Author

lmapii commented Jun 14, 2022

Sorry, providing more information: My test runner is created manually, it runs within an OS task:

void Task00(...)
{
    // <snip>
    // UNITY_BEGIN() is called from the startup hooks
    RUN_TEST(TestDopFun);
    RUN_TEST(TestDopOwner);
    RUN_TEST(TestPwmOwner);
    RUN_TEST(TestPwmFun);
    RUN_TEST(TestLpoOwner);
    RUN_TEST(TestLpoFun);
    UNITY_END();
    __asm("debug");
}

When everything is OK I'm seeing all results correctly (handled via the debugger interface):

../Application/TestOwner/src/TestDop.c:50:TestDopFun:PASS
../Application/TestOwner/src/TestDop.c:51:TestDopOwner:PASS
../Application/TestOwner/src/TestPwm.c:52:TestPwmOwner:PASS
../Application/TestOwner/src/TestPwm.c:53:TestPwmFun:PASS
../Application/TestOwner/src/TestLpo.c:54:TestLpoOwner:PASS
../Application/TestOwner/src/TestLpo.c:55:TestLpoFun:PASS

-----------------------
6 Tests 0 Failures 0 Ignored 
OK

If, say, TestDopOwner fails, I'm seeing the following output:

../Application/TestOwner/src/TestDop.c:50:TestDopFun:PASS
../Application/TestOwner/src/TestDop.c:120:TestDopOwner:FAIL: Expected 0 Was 1

Only if I use #define UNITY_EXCLUDE_SETJMP_H in my unity_config.h I'm seeing the full output.

I think the main gotcha is that my UNITY_BEGIN is not called in the task context. I can't do that since the communication with the debugger must be initialized by a startup hook since it is blocking the execution long enough for my OS ticks to break (the execution would only start after a timeout).

If I'm using #define UNITY_EXCLUDE_SETJMP_H the execution uses return and thus continues in the task context and not in the hook.

@mvandervoord
Copy link
Member

Thanks. This is all good information. You're using the default RUN_TEST macro, I assume?

I don't believe there should be a problem with calling UNITY_BEGIN() from a different task, so long as you're confident it gets called before the first test. It's not handling any of the abortframe management for setjmp, so the issue of it using a different stack shouldn't be an issue.

I don't have a better guess at this point, unless you've customized RUN_TEST. You could consider initializing the actual communication from where you are now, and then assigning the UNITY_OUTPUT_START() macro to nothing so that you can call it from the test task with everything else?

@lmapii
Copy link
Author

lmapii commented Jun 14, 2022

My unity configuration is the following

#define UNITY_OUTPUT_CHAR(a)    UnityOutPut(a)
#define UNITY_OUTPUT_START()    UnityOutInit()
#define UNITY_OUTPUT_FLUSH()    UnityOutFlush()
#define UNITY_OUTPUT_COMPLETE() UnityOutClose()

#define UNITY_EXCLUDE_LIMITS_H
#define UNITY_EXCLUDE_STDDEF_H
#define UNITY_INCLUDE_PRINT_FORMATTED

#define UNITY_INT_WIDTH     32
#define UNITY_LONG_WIDTH    32
#define UNITY_POINTER_WIDTH 32

#define UNITY_EXCLUDE_FLOAT
#define UNITY_EXCLUDE_FLOAT_PRINT

// TODO: prevent Unity from BAILING on failures https://github.com/ThrowTheSwitch/Unity/issues/616
// #define UNITY_EXCLUDE_SETJMP_H

So I'm really not customizing RUN_TEST. I've tried isolating the initialization of the communication but as you mentioned it doesn't change anything.

My current fear is that the longjmp instruction is not really working for the architecture (I'm on an Infineon TriCore). In my implementation, the log transfer stops when the execution of the CPU stops, which also happens in case of a trap. I'm currently debugging that and I'll check the architecture manual for longjmp.

@mvandervoord
Copy link
Member

It IS sounding like setjmp/longjmp itself is the problem. Optimistically, I'd say that if they include the setjmp library, that means they support it... but it's also one of those libraries that doesn't get the attention that the other standard libraries do.

It sounds like you are in a multitasking environment, which complicates things slightly. It is important that all interactions with setjmp / longjmp happen from the same task (and therefore stack). From everything you've written, though, it sounds like you have that under control already. I'm writing this so (a) it might jog your memory of something that might be a little different and (b) anyone coming to this thread later understands it's important. ;)

I think I've run out of helpful things to say at the moment. If you learn anything or have questions, dump it here. Good luck! I'll be interested to see what's going on.

@lmapii
Copy link
Author

lmapii commented Jun 14, 2022

Yeah, it is definitely not ideal :D. The problem is that the execution of the longjmp implementation requires supervisor mode (due to the mtcr instruction) and my test task needs to run in a user mode. I'm ending up in a trap (Privileged Instruction just for anyone seeing the same). I'd need to be able to provide wrapper functions for TEST_PROTECT and TEST_ABORT, but these are defined as follows:

#define TEST_PROTECT() (setjmp(Unity.AbortFrame) == 0)
#define TEST_ABORT() longjmp(Unity.AbortFrame, 1)

I have my own clone of the unity repository so I can try to add a workaround here and maybe add a pull request such that these definitions could be provided by the unity_config.h, but maybe you already have an idea how this could be done in a clean way?

I'm not really blocked since I'm wrapping unity in pytest (to automate re-flashing etc.) and have a consistency check for the number of executed tests anyhow, so I'm ending up in a complete failure if the number of executed tests is not the same. Also, for now, I could simply keep on excluding longjmp since I'm not using any kind of mocking.

@mvandervoord
Copy link
Member

Ah. this makes a lot of sense.

Long-term, it probably makes sense to allow people to customize TEST_PROTECT and TEST_ABORT. I'm glad this isn't holding you back and that you've found an answer that makes sense!

@lmapii
Copy link
Author

lmapii commented Jun 14, 2022

Yeah, for now, I'm leaving my integration as follows:

// TODO: https://github.com/ThrowTheSwitch/Unity/issues/616
// The implementation of `longjmp` uses `mtcr` which requires supervisor privileges.
// Therefore, for now, we're excluding SETJMP such that the normal `return` instruction is used instead.
#define UNITY_EXCLUDE_SETJMP_H

Then my output is fine:

../Application/TestOwner/src/Task00.c:57:TestDopFun:PASS
../Application/TestOwner/src/Task00.c:120:TestDopOwner:FAIL: Expected 0 Was 1
../Application/TestOwner/src/Task00.c:59:TestPwmOwner:PASS
../Application/TestOwner/src/Task00.c:60:TestPwmFun:PASS
../Application/TestOwner/src/Task00.c:61:TestLpoOwner:PASS
../Application/TestOwner/src/Task00.c:62:TestLpoFun:PASS

-----------------------
6 Tests 1 Failures 0 Ignored 
FAIL

Should I keep this issue open?

@mvandervoord
Copy link
Member

Yes, let's keep it open. Memo to myself or whoever gets here first: Make TEST_PROTECT and TEST_ABORT redefineable. Also, add documentation for why you may care to do this (hint, read the entries above). :)

@lmapii
Copy link
Author

lmapii commented Jun 14, 2022

Thanks, also for the superfast feedback! I'm very happy with unity, especially because it is not a blown-up solution and therefore so easy to integrate.

@mvandervoord
Copy link
Member

Thanks! I appreciate the kind words. You've identified our big goal there! I'm glad to know we're near the target!

@nordic-krch
Copy link

@mvandervoord anything new about customizing TEST_ABORT?

In our case, we are using Unity for on target tests and use longjmp version of TEST_ABORT. However, some tests may fail in the interrupt context and in that case we don't want longjmp but rather return (otherwise next test executes from that interrupt context). We had to modify TEST_ABORT to if (is_in_isr()) return; else longjmp().

@lmapii
Copy link
Author

lmapii commented Apr 27, 2024

The merged fix for issue #695 should solve this issue as well :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants