search

tcunit / TcUnit

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

Measure overall, testsuite and test duration #159

Closed stefanbesler closed 1 year ago

stefanbesler commented 3 years ago

This PR extends to code in a way to fill the "time" attribute of the junit xml specification. I haven't tested this extensively, but just want to let you know that I am on this and maybe you have some opinions and could do a code review on it? I am not completely fluent with TcUnit's structure and maybe there are better ways to include time measurements.

I am not sure if I have enough time at the moment for this as a side project, but in the end I want to extend TcUnit with benchmark capabilities. At the moment, one of my hobby projects is porting a trajectory generator to TwinCAT ( github project: struckig ) and I am using TcUnit for Unittesting . I think it would make sense to use it in a benchmark context as well by including test iterations and stuff like that (similar to what I have done without a library here (https://github.com/stefanbesler/struckig/tree/main/benchmark) ). My goal is to see potential regressions by utilizing a benchmark github action.

sagatowski commented 3 years ago

Hi! Sorry for late reply. Thanks for looking into this. I really like this function. Is it possible that you could use the latest TwinCAT 3.1.4022.x to do the changes though? (according to https://github.com/tcunit/TcUnit/blob/master/CONTRIBUTING.md).

What is the duration measured in? MS? I guess that if we want to have this included in the JUnit XML-file we will also need to update the TcUnit-Runner.

Did you try to run the TcUnit-Verifier with the new version of TcUnit, just to check that no functionality has been broken?

Edit: Cool initiative with struckig! I might use this myself.

stefanbesler commented 2 years ago

so, half a year later ... :-)

Is it possible that you could use the latest TwinCAT 3.1.4022.x to do the changes though?

done, tested it on 4022.29

What is the duration measured in? MS?

it was in microseconds before, but I changed to seconds, which is according to junit's specification or at least how the Jenkins plugins interprets it

I guess that if we want to have this included in the JUnit XML-file we will also need to update the TcUnit-Runner.

Well, I would have prefered to only consider the duration in the xml output of TcUnit, but anyway ... I am on it, I will make a pull request in TcUnit-Runner as well

Did you try to run the TcUnit-Verifier with the new version of TcUnit, just to check that no functionality has been broken?

I ran TcUnit-Verifier, but I had to modify some stuff to get it working (change IDE and Twincat Version that is verified with), would you run it on your setup as well please?

sagatowski commented 2 years ago

Sorry for late response, been really busy lately... I will review this, but one thing I noticed is that there are no tests added for this functionality into the TcUnit-Verifier. Is it possible to add a few tests to make sure that you get the expected output?

stefanbesler commented 2 years ago

I added and changed some tests for the durations functionality

sagatowski commented 1 year ago

Better late than never. I did some minor corrections to your PR. Thank you so much for your contribution @stefanbesler.

sagatowski commented 1 year ago

@stefanbesler Note that this new feature doesn't work with sequenced tests. Check this issue: https://github.com/tcunit/TcUnit/issues/210. Would you have time to look into it?

sagatowski commented 1 year ago

@stefanbesler I've also found a second issue with this PR. See #211.

sagatowski commented 3 months ago

@stefanbesler This PR is still haunting us... See: