tcunit / TcUnit

An unit testing framework for Beckhoff's TwinCAT 3
Other
271 stars 74 forks source link

Return GVL_TcUnit.CurrentTestIsFinished from TEST() #162

Closed oswin02 closed 1 year ago

oswin02 commented 3 years ago

Is it a bad idea to return the Value of GVL_TcUnit.CurrentTestIsFinished from the TEST() function to allow skipping all the following code in the test? I would then start a test like:

IF TEST('MyTest') THEN RETURN; END_IF
sagatowski commented 3 years ago

Hi!

You can't do an IF on the test right now as it's not returning any value.

You can use the function: IS_TEST_FINISHED()

Maybe this will fulfill your needs?

oswin02 commented 3 years ago

Hi, This is exactly what I do now, but I find it more convenient if the TEST function itself could return this value.

sagatowski commented 3 years ago

I guess it makes sense. Would this affect back-ward compability in any way?

I'm thinking TE1200 static code analysis. If we add a returning boolean to TEST(), then tests of typ "SA0009: Unused return values" would suddenly fail for everyone that is using this tool.

image

sagatowski commented 3 years ago

There is actually another disadvantage. The function IS_TEST_FINISHED() would have to be called for all tests, which does:

TestName := F_LTrim(in := F_RTrim(in := TestName));

IS_TEST_FINISHED := FALSE;

FOR Counter := 1 TO GVL_TcUnit.CurrentTestSuiteBeingCalled^.GetNumberOfTests() BY 1 DO
    CurrentTest := GVL_TcUnit.CurrentTestSuiteBeingCalled^.Tests[Counter];
    IF CurrentTest.TestName = TestName THEN
        IS_TEST_FINISHED := CurrentTest.IsFinished();
        RETURN;
    END_IF
END_FOR

Which could take a lot of time if you have a lot of tests. This would mean that the user would not have an option to run this or not (unless we added a parameter) making the tests slower. It could be argued that the time could be saved by not having to run the function blocks under test and doing asserts instead though. There is now an issue that tests are slower in TcUnit 1.2 than 1.0, which makes me worried not to make matters worse.

oswin02 commented 3 years ago

You could simply return GVL_TcUnit.CurrentTestIsFinished, so that's not a problem. But I understand the concern about static code analysis.

sagatowski commented 3 years ago

@oswin02 That's true! Let me think a little bit through it. The advantages probably outweigh the one disadvantage of TE1200.

oswin02 commented 3 years ago

IS_TEST_FINISHED could also be simplified in just returning GVL_TcUnit.CurrentTestIsFinished. I do not unit test in other languages, but is it not a bad behaviour when the code is still executed after the test is completed? In this case the error in static code analysis is reasonable.

sagatowski commented 2 years ago

With the current architecture of TcUnit, then yes, the tests are still executed after the tests are completed. It's not a huge problem though as once the tests are completed, you can simply stop the TwinCAT runtime. In a future version, this could be changed, but then this would most likely break the API.

sagatowski commented 2 years ago

Ok to close this issue?

sagatowski commented 11 months ago

Related to #124.