[CI] Get coverage to report with SonarCloud

[da1910]

With some sed magic the coverage reports are now actually parsed and uploaded. There is a warning about the <sources> element, since the path is incorrect, but this does not seem to cause problems. I did look at removing the element entirely, since it's optional according to the DTD however editing XML with sed is dangerous.

It might be worth farming this out to a bash script and maybe some XSLT, we could also look at getting a switch added to coverage.py to disable the sources element entirely.

[Andy-Grigg]

I'm on the fence about this.

Generally speaking, I think it makes sense to scan test code with (almost) the same rigor as you do production code. The failure mode is a bit more indirect, but it's possible that a bug in a test could cause a bug in the production code to be missed, and avoiding that testing bug would mean you catch the real bug.

Sonar seems to be doing multiple things though.

For coverage, I think there's benefit in including the test code in the output. Apart from possibly some platform-specific functionality, the test code should generally have 100% coverage. If not, it might be because of a mistake (returning early, not starting with 'test_', etc.).

For general code quality/security though, I agree it should be skipped. All the security issues are related to test code which are creating ephemeral servers/objects in secure environments and so are perfectly safe. This should be ignored.

To summarize, I think ignoring tests is fine from a security perspective, but it would be nice to be able to still get coverage reports for it if possible.

[da1910]

I'm on the fence about this.

Generally speaking, I think it makes sense to scan test code with (almost) the same rigor as you do production code. The failure mode is a bit more indirect, but it's possible that a bug in a test could cause a bug in the production code to be missed, and avoiding that testing bug would mean you catch the real bug.

Sonar seems to be doing multiple things though.

For coverage, I think there's benefit in including the test code in the output. Apart from possibly some platform-specific functionality, the test code should generally have 100% coverage. If not, it might be because of a mistake (returning early, not starting with 'test_', etc.).

For general code quality/security though, I agree it should be skipped. All the security issues are related to test code which are creating ephemeral servers/objects in secure environments and so are perfectly safe. This should be ignored.

To summarize, I think ignoring tests is fine from a security perspective, but it would be nice to be able to still get coverage reports for it if possible.

In which case we can manually mark the issues as safe if they're specific to "this test is testing insecure behaviour", that's not an unreasonable burden, and as you say there are benefits.

I do still need to work out why it's not reporting test coverage though.