search

git-afsantos / haros

H(igh) A(ssurance) ROS - Static analysis of ROS application code.
MIT License
191 stars 37 forks source link

JUnit XML output #79

Closed max-krichenbauer closed 5 years ago

max-krichenbauer commented 5 years ago

In preparation for possible future use of HAROS in the ROS build farm (as a package to be requested by package developers through the build system), I added JUnit XML format file output.

gavanderhoorn commented 5 years ago

This is a nice PR, but I think we'd need to discuss whether this is something that should be added to Haros itself, or left to consumers of its output.

Having it integrated obviously makes it easier for users, as they can just configure Haros to output this. Otherwise they'd have to do it themselves.

I would suggest to make this configurable using a command line option (ie: whether junit xml should be output).

max-krichenbauer commented 5 years ago

Thanks for the comments! The thing is that JUnit output can (and possibly should) include some data that cannot be recovered from the normal JSON output. In this PR, only the time that it took to create the report was added and the total number of tests (rules) is included - both are not in the JSON output. So since JUnit is a quite common standard, in the mid-to-long time frame it may be useful to HAROS users to add more reporting logic into HAROS to create complete JUnit XML files.

Having a command line option --xml to manually enable this output is a very good idea though. Will add it ASAP.

max-krichenbauer commented 5 years ago

Both (the --xml flag and catching exceptions) added in the last commit.

gavanderhoorn commented 5 years ago

The thing is that JUnit output can (and possibly should) include some data that cannot be recovered from the normal JSON output.

this can of course always be fixed by just adding those to the .json files.

max-krichenbauer commented 5 years ago

The thing is that JUnit output can (and possibly should) include some data that cannot be recovered from the normal JSON output.

this can of course always be fixed by just adding those to the .json files.

True, it would be possible to create a wholly new project "HAROS-output-to-JUnit-XML-converter", but:

1) then I would have to make PRs to HAROS asking for JSON output changes to satisfy the needs of this one external tool ("please add this output to JSON files so my tool works"). 2) it's also quite a round-about way to write files to disk first, then parse them again to convert them. Inside HAROS the data is already prepared. 3) people using HAROS will find it more easily. It's unlikely that people who are interested in this feature Google "HAROS to JUnitXML converter.

git-afsantos commented 5 years ago

@max-krichenbauer if you would address the two remaining comments I left previously, then I think this is ready to merge.

finally does not make sense here. It will log "failed to write" even when everything goes well.

close() is still not enclosed within either a finally or a with statement on the respective open.

max-krichenbauer commented 5 years ago

Ah, sorry - I missed those. Fixed now.

git-afsantos commented 5 years ago

This is now merged, but I made a couple of small adjustments. I moved the --junit-xml-output option to individual commands, because it was missing in the new parse command, and viz does not export any files.

This is not released yet. I am working on a couple more changes of my own, so I will just release everything at once in v3.8.

max-krichenbauer commented 5 years ago

Thanks for the merge! I noticed a small bug I have pointed out here: https://github.com/git-afsantos/haros/commit/2b99524b519ed7f6f5a5954c0d80bc41af744cb3#r35013737 Basically HarosVizRunner is broken right now, because it does not specify the value for junit_xml_output in the parent's constructor, which doesn't have a default value.