tcunit / TcUnit

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

Incorrect number of failed tests and successful tests if too many tests in a test suite. #132

Closed Beidendorfer closed 3 years ago

Beidendorfer commented 3 years ago

Incorrect number of failed tests and successful tests if too many tests in a test suite. If the tests in a test suite are greater than the maximum number of tests per test suite, the evaluation is incorrect

See picture: the same test gives different results. It should have 100 passed and 116 failed test image

Test outside of the Test[] Array (>100) get analysis. The results point to some memory area image

I see three Solutions

1.) Limit the test to the maximum size. GetNumberOfTests() Problem if there are more tests than 100 (default parameter) there is no indication that some tests were not evaluated. Adjustment of the error of the error message would be necessary

2.)Restrict all methods in Test_Suit to their array bounds.

**Problem** here is that the tests >100 are not executed and thus all 216 tests are marked as passed. This is because the 
calculation of the passed test is "Number of Test - failed Test".
e.g. 216 test - 0 failed test = 216 passed test

Test >100 are not evaluated and can not fail.

Here the faildTest counter could be preset to the value greater than 100 (difference 216-100) = 116 all further faild test would be added.

3.) Combination of 1&2

I think the bug only occurs when the test > max test. But since it can happen. we should catch the bug

I prefer the 3 solution even if it would be already fixed with solution 1. I like to limit the range of values in for loops for arrays to their limits

sagatowski commented 3 years ago

Why is there a duplicate output in first picture? And why are they different?

I thought this issue is mostly solved already by the commit to solve this issue? https://github.com/tcunit/TcUnit/issues/128 no?

Also, we could simply set all tests to fail after the 100? To be sure that the message is evaluated...

Beidendorfer commented 3 years ago

Why is there a duplicate output in first picture?

I run the same Test two times and I disable the Messages(only Error are shown)

And why are they different? When 100 test are finished AllTestfinished is set In parallel the for loop for the test >100 is still running and depending on how

fast the Ads Logger outputs the result the number of passed and failed test changes.

I thought this issue is mostly solved already by the commit to solve this issue?

128

no?

128 Generates an error message if there are more tests than configured.

After further testing I found out that the number of passed and failed tests varies for the same test suite. I noticed that in some methods the test array limits are exceeded.

As a programmer I would like to know if all tests were executed correctly and if there are problems e.g. exceeding the test, an error log would be helpful to bring me to the solution of the problem.

Therefore I would like to keep the error message and to avoid further error cases limit the test arrays.

I think I could implement a possible solution

sagatowski commented 3 years ago

@Beidendorfer Thanks for your fast response, now I understand.

As a programmer I would like to know if all tests were executed correctly and if there are problems e.g. exceeding the test, an error log would be helpful to bring me to the solution of the problem.

Therefore I would like to keep the error message and to avoid further error cases limit the test arrays.

I think I could implement a possible solution

Alright, sounds good! Just make sure to follow the following: https://github.com/tcunit/TcUnit/blob/master/CONTRIBUTING.md

sagatowski commented 3 years ago

@Beidendorfer It's described in: https://github.com/tcunit/TcUnit/tree/master/TcUnit-Verifier

If you can think of any test cases to add to the TcUnit-Verifier, it would be good!

sagatowski commented 3 years ago

Solved in ce84260.