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

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

Open
TobiasKnauss opened this issue Oct 4, 2024 · 5 comments

Comments

@TobiasKnauss
Copy link

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

  • TwinCAT XAE 3.1.4026.10 (latest)
  • VS 2022 (not so relevant, XAE is used)
  • TcUnit 1.3.1

Run environment
Laptop

Additional context
None that seems relevant.

@TobiasKnauss
Copy link
Author

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
Copy link
Author

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
Copy link
Member

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
Copy link
Member

sagatowski commented Oct 4, 2024

...
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
Copy link
Author

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. ;-)

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

2 participants