Open slacAWallace opened 3 years ago
Hi @slacAWallace & thanks for this great suggestion! Although I think your suggestion makes a lot of sense (I too have tests that can take a lot of time to finish), I'm not sure how this should work. Or at least it raises some questions. Just a few questions to initiate the discussion:
IS_TEST_FINISHED()
), maybe something like TEST_STATUS() could be added that returns an enumeration (FAILED, SUCCESSFUL, RUNNING)?Also, it would be interesting to see how other unit testing frameworks handle this (if they do at all).
Regarding (1) I think the default behaviour should always be to finish all tests independently of whether one test fails or not. An option could be to add a parameter to the TcUnit-framework to enable a feature that stops running of the tests if one test fails. By using parameters, the user could configure each use of TcUnit for every library as the parameters for library references are stored in the project itself.
@slacAWallace Would it be a good solution/option for you to have a parameter that you can set (through the GVL_Param_TcUnit), that if it is set, if an assertion fails, the rest of the tests are skipped (not enabled by default)?
@slacAWallace Thank you for this question. I have a requirement as mentioned below.
test := AssertEquals_BOOL
IF test THEN
ADSLOGSTR(
msgCtrlMask := ADSLOG_MSGTYPE_HINT,
msgFmtStr := 'Test message. Parameter is %s',
strArg := 'ABC'
);
END_IF
Watching this space for the solution.
Many unit testing frameworks will raise a specific exception within ASSERT to prevent a test from continuing, but to still execute other tests. I would like to see this added, although it would likely be a breaking change and may require v2.0. It could possibly be added as an option controlled by a BOOL parameter for v1.3?
Edit: It looks like exception handling (TRY/CATCH) is only supported on 32-bit systems at the moment, so this may not be practical yet.
@kumaraswamygaviyappa All Assert-function provides a message-field, so that you can enter your own message if an assertion should fail. If an assertion fails, the message will be displayed.
@RodneyRichardson One option could be to add a parameter to TcUnit, to stop running the tests if one assertion fails, though I'm not sure this is what @slacAWallace wants?
@RodneyRichardson One option could be to add a parameter to TcUnit, to stop running the tests if one assertion fails, though I'm not sure this is what @slacAWallace wants?
That's right, I am hoping to get something a bit more local. I have some test patterns I use that keep a test open for multiple cycles, until a particular condition is met (a counter value for example). If in the middle of these tests an assert fails, then there's often no reason to keep running through the rest of the asserts. I'll try to sketch this up soon with a PR.
Moved this to Release 1.3.0.0. Looking forward to your PR @slacAWallace (don't forget to read the CONTRIBUTING document).
I sat down over the last weekend to prepare something for this and ended up just thinking. After some discussion with a colleague yesterday I think I understand @sagatowski 's original response more completely now.
Should I prepare a PR to investigate the change in behavior?
On 1) I could imagine that this behaviour could be enabled or disabled by the user. I.e. some global variable to either continue testing after a fail or stop testing after a fail. If you keep it testing after fail this defaults to current behaviour.
Als could you see if you can easily implement some way to reset the framework and restart the tests without a new download or cold reset of the PLC? I'd love to see a second opinion on this!?
Thank you in advance.
Edit: PS The Parameterlist is phased out from CODESYS since SP16 or so. Usage is still possible but the meantime no adequate alternative is given so we must stick with it for now. An alternative will come in SP18 but that will take at least a small year and thus even a bit longer for TwinCAT to follow up.
@slacAWallace You're welcome to do it, but take into consideration this issue. If this change is done, you could instead simply do
IF TEST('Blahonga') THEN
// Only execute FB under test and do assertions here
END_IF
The IF would be optional, not to break any existing tests to be 100% backward compatible with tests written in prior versions of TcUnit. I haven't dig deeper into the above issue yet though, and haven't investigated whether this would truly work without breaking any existing usages.
Also, don't forget CONTRIBUTING.
@HAHermsen Why were ParameterList removed (without having any alterantive)? They are quite commonly used. You can implement the suggested function in CfUnit, and if you find it works well we can back-port it into TcUnit.
@HAHermsen Why were ParameterList removed (without having any alternative) They are quite commonly used => So True! I was kind of surprised when I found out they are not supported anymore. CODESYS decided to "silently drop" them from SP16 onwards (we are at 17 now). The exact reason is unknown to me. For now they are still usable in CODESYS projects but we cannot insert them in a new project, since the object has disappeared from the add object list dialog. A workaround is to export a Parameter List from an older IDE/project and re-import it into a new project. I was told that a alternative will follow suit but we must wait for SP18 for it. As soon as I receive more info, I'll drop a message here.
EDIT PS cfunit has changed name to co♻e: A unittest framework for CODESYS (in short coUnit), because of obvious reasons (co = codesys, tc = twincat). I only wished we came up with this name a year earlier ;-)
PSS We also use git now since the launch of SP17, CODESYS IDE has a (paid) git client. Its fairly recent so not as feature rich yet but it manages quite nice.
No activity in a while. I'll move this as an idea for TcUnit 2.0, as I've seen some projects using asserts in such a way that they DON'T expect a boolean to return as this would break their static code analysis.
For anyone that comes across this and needs an urgent solution, you can call IS_TEST_FINISHED()
in your state-machine when you are doing your assert (if you have multiple asserts, which I assume is the case here). If this returns true, simply don't do any more asserts. It's not as clean solution as getting a boolean directly, but it will work.
I am finding myself wishing for some kind of result from an assert method to tell me if the assertion passed/failed. Some of my test setups use a loop and if one assertion fails there is little to no point in running through the rest.