tcunit / TcUnit

An unit testing framework for Beckhoff's TwinCAT 3
Other
273 stars 75 forks source link

Complete assert message is not shown if the TcUnit part exceeds approx 250 chars #250

Open TobiasKnauss opened 1 month ago

TobiasKnauss commented 1 month ago

Describe the bug The complete assert message is not shown if it exceeds a certain length. It is not cut-off, but just omitted.

To Reproduce

expectedValue0 := 'A';
actualValue0   := 'B';

AssertEquals (Expected := expectedValue0,
              Actual   := actualValue0,
              Message  := 'FAIL_1_FAIL_1_FAIL_1_FAIL_1_FAIL_1_FAIL_1_FAIL_1_FAIL_1_FAIL_1__Len69');
AssertEquals (Expected := expectedValue0,
              Actual   := actualValue0,
              Message  := 'FAIL_2_FAIL_2_FAIL_2_FAIL_2_FAIL_2_FAIL_2_FAIL_2_FAIL_2_FAIL_2_FAIL_2_FA__Len79');
AssertEquals (Expected := expectedValue0,
              Actual   := actualValue0,
              Message  := 'FAIL_3_FAIL_3_FAIL_3_FAIL_3_FAIL_3_FAIL_3_FAIL_3_FAIL_3_FAIL_3_FAIL_3_FAIL__Len81');
AssertEquals (Expected := expectedValue0,
              Actual   := actualValue0,
              Message  := 'FAIL_4_FAIL_4_FAIL_4_FAIL_4_FAIL_4_FAIL_4_FAIL_4_FAIL_4_FAIL_4_FAIL_4_FAIL_4_FAIL_4_FAIL_4_FAIL_4_FAIL_4_FAIL_4_FAIL_4__Len126');
AssertEquals (Expected := expectedValue0,
              Actual   := actualValue0,
              Message  := 'FAIL_5_FAIL_5_FAIL_5_FAIL_5_FAIL_5_FAIL_5_FAIL_5_FAIL_5_FAIL_5_FAIL_5_FAIL_5_FAIL_5_FAIL_5_FAIL_5_FAIL_5_FAIL_5_FAIL_5___Len127');

Output:

Error       04.10.2024 13:01:13 978 ms | 'PlcTask' (350): FAILED TEST 'PRG_TEST.m_TestSuiteFb_CreateFailureMessage@Test_CreateFailureMessage_ValueOutOfRange_1', EXP: A, ACT: B, MSG: FAIL_1_FAIL_1_FAIL_1_FAIL_1_FAIL_1_FAIL_1_FAIL_1_FAIL_1_FAIL_1__Len69             
Error       04.10.2024 13:01:13 998 ms | 'PlcTask' (350): FAILED TEST 'PRG_TEST.m_TestSuiteFb_CreateFailureMessage@Test_CreateFailureMessage_ValueOutOfRange_1', EXP: A, ACT: B, MSG: FAIL_2_FAIL_2_FAIL_2_FAIL_2_FAIL_2_FAIL_2_FAIL_2_FAIL_2_FAIL_2_FAIL_2_FA__Len79               
Error       04.10.2024 13:01:14 018 ms | 'PlcTask' (350): FAILED TEST 'PRG_TEST.m_TestSuiteFb_CreateFailureMessage@Test_CreateFailureMessage_ValueOutOfRange_1', EXP: A, ACT: B, MSG: FAIL_3_FAIL_3_FAIL_3_FAIL_3_FAIL_3_FAIL_3_FAIL_3_FAIL_3_FAIL_3_FAIL_3_FAIL__Len81             
Error       04.10.2024 13:01:14 038 ms | 'PlcTask' (350): FAILED TEST 'PRG_TEST.m_TestSuiteFb_CreateFailureMessage@Test_CreateFailureMessage_ValueOutOfRange_1', EXP: A, ACT: B, MSG: FAIL_4_FAIL_4_FAIL_4_FAIL_4_FAIL_4_FAIL_4_FAIL_4_FAIL_4_FAIL_4_FAIL_4_FAIL_4_FAIL_4_FAIL_4_FAIL_4_FAIL_4_FAIL_4_FAIL_4__Len126                
Error       04.10.2024 13:01:14 218 ms | 'PlcTask' (350): | ==========TESTS FINISHED RUNNING==========              
Error       04.10.2024 13:01:14 238 ms | 'PlcTask' (350): | Test suites: 1              
Error       04.10.2024 13:01:14 258 ms | 'PlcTask' (350): | Tests: 1                
Error       04.10.2024 13:01:14 278 ms | 'PlcTask' (350): | Successful tests: 0             
Error       04.10.2024 13:01:14 298 ms | 'PlcTask' (350): | Failed tests: 1             
Error       04.10.2024 13:01:14 318 ms | 'PlcTask' (350): | Duration: 1.059e-4              
Error       04.10.2024 13:01:14 338 ms | 'PlcTask' (350): | ======================================              
Message     04.10.2024 13:01:14 078 ms | 'PlcTask' (350): | Test suite ID=0 'PRG_TEST.m_TestSuiteFb_CreateFailureMessage'               
Message     04.10.2024 13:01:14 098 ms | 'PlcTask' (350): | ID=0 number of tests=1, number of failed tests=1, duration=9.7e-5               
Message     04.10.2024 13:01:14 118 ms | 'PlcTask' (350): | Test name=Test_CreateFailureMessage_ValueOutOfRange_1               
Message     04.10.2024 13:01:14 138 ms | 'PlcTask' (350): | Test class name=PRG_TEST.m_TestSuiteFb_CreateFailureMessage             
Message     04.10.2024 13:01:14 158 ms | 'PlcTask' (350): | Test status=FAIL, number of asserts=5, duration=8.74e-5             
Message     04.10.2024 13:01:14 178 ms | 'PlcTask' (350): | Test assert message=FAIL_1_FAIL_1_FAIL_1_FAIL_1_FAIL_1_FAIL_1_FAIL_1_FAIL_1_FAIL_1__Len69               
Message     04.10.2024 13:01:14 198 ms | 'PlcTask' (350): | Test assert type=STRING                 

If the texts of Expected and Actual are longer, the possible message length becomes shorter.

Expected behavior See the message with text "FAIL_5_FAIL_5_FAIL_5_FAIL_5_FAIL_5_FAIL_5_FAIL_5_FAIL_5_FAIL_5_FAIL_5_FAIL_5_FAIL_5_FAIL_5_FAIL_5_FAIL_5_FAIL_5_FAIL_5___Len1", either in full length of cut off.

Software versions

Run environment Laptop

Additional context None that seems relevant.

TobiasKnauss commented 1 month ago

Du to this limitation in message length, it would be nice if you changed the Assert methods to return a boolean value as a quick workaround.
This return value could be used to print more messages with details. Otherwise, an own, duplicate check has to be introduced, which will probably return a different result than the Assert methods. Example:

IF (NOT AssertEquals (Expected := expectedValue0,
                      Actual   := actualValue0,
                      Message  := 'expectedValue0 <> actualValue0')) THEN
    TCUNIT_ADSLOGSTR (ADSLOG_MSGTYPE_ERROR, 'expectedValue0 = %s', expectedValue0);
    TCUNIT_ADSLOGSTR (ADSLOG_MSGTYPE_ERROR, 'actualValue0   = %s', actualValue0);
END_IF
TobiasKnauss commented 1 month ago

Another idea for a workaround (I still would like to see the bool Assert...() though): If the message is too long, wrap it to the next message.

FAILED TEST 'PRG_TEST.m_TestSuiteFb_CreateFailureMessage@Test_CreateFailureMessage_ValueOutOfRange_1', EXP: A, ACT: B, MSG: FAIL_5_FAIL_5_FAIL_5_FAIL_5_FAIL_5_FAIL_5_FAIL_5_FAIL_5_FAIL_5_FAIL_5_FAIL_5_FAIL_5_FAIL_5_FAIL_5_FAIL_5_FAIL_5_FAIL_5___Len12
FAILED TEST 'PRG_TEST.m_TestSuiteFb_CreateFailureMessage@Test_CreateFailureMessage_ValueOutOfRange_1', MSG continued: 7

The same could be done if the wrapping would already have to be done in EXP or ACT.
Then you could see the content of long strings (up to 255 chars) and of a long message (also up to 255 chars), probably split up into 5 or 6 lines.

sagatowski commented 1 month ago

Du to this limitation in message length, it would be nice if you changed the Assert methods to return a boolean value as a quick workaround. This return value could be used to print more messages with details. Otherwise, an own, duplicate check has to be introduced, which will probably return a different result than the Assert methods. Example:

IF (NOT AssertEquals (Expected := expectedValue0,
                      Actual   := actualValue0,
                      Message  := 'expectedValue0 <> actualValue0')) THEN
  TCUNIT_ADSLOGSTR (ADSLOG_MSGTYPE_ERROR, 'expectedValue0 = %s', expectedValue0);
  TCUNIT_ADSLOGSTR (ADSLOG_MSGTYPE_ERROR, 'actualValue0   = %s', actualValue0);
END_IF

We can't change the return-value of the assert as that is a change in the API. For instance, everyone doing an assert (100% of the TcUnit users) would get a TE1200 / SA0009 error. Our "quick" fix would turn into a horror-show for all the users using the static code analysis.

sagatowski commented 1 month ago

... Expected behavior See the message with text "FAIL_5_FAIL_5_FAIL_5_FAIL_5_FAIL_5_FAIL_5_FAIL_5_FAIL_5_FAIL_5_FAIL_5_FAIL_5_FAIL_5_FAIL_5_FAIL_5_FAIL_5_FAIL_5_FAIL_5___Len1", either in full length of cut off.

@TobiasKnauss Agree with the expected behaviour.

PR welcome. Full length is not possible as we are dependent on ADSLOGSTR(). Don't forget to read https://github.com/tcunit/TcUnit/blob/master/CONTRIBUTING.md.

TobiasKnauss commented 1 month ago

We can't change the return-value of the assert as that is a change in the API. For instance, everyone doing an assert (100% of the TcUnit users) would get a TE1200 / SA0009 error. Our "quick" fix would turn into a horror-show for all the users using the static code analysis.

@sagatowski I already thought of creating a PR with changes to return BOOL values. You're right, changing the existing API is not an option, so I would simply add another set of functions that internally call each other.

I don't have the TE1200 license yet. Would it work to call the new Assert...() : BOOL from the existing Assert...() (void) inside TcUnit without getting SA0009? The return value could be assigned to a temporary variable that would just be discarded in the end, to satisfy the compiler.

Don't forget to read https://github.com/tcunit/TcUnit/blob/master/CONTRIBUTING.md.

The matter of contributing... to the football club, to the fire fighters, to the job and family and friends... The day has 24 hours plus the night. ;-)