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

CxxXunitSensor in a multimodule project #612

Closed
Zetten opened this issue Aug 18, 2015 · 7 comments
Closed

CxxXunitSensor in a multimodule project #612

Zetten opened this issue Aug 18, 2015 · 7 comments
Assignees
Milestone

Comments

@Zetten
Copy link
Contributor

Zetten commented Aug 18, 2015

We've realised (after not paying attention for a while) that we're not getting test execution reports for our C++ tests. It's not a vital feature of our SQ usage, but it used to work and I'm trying to narrow down why it doesn't now.

I'm not sure of the intent behind f2cedf7 and the general idea behind overriding shouldExecuteOnProject in the CxxXunitSensor. The commit message from @jmecosta introducing the override just states "prevent simple mode from running in all modules" and I can't figure out why that's desirable.

As it stands, we can't get simple test execution reports in our multi-module project by any means. I haven't yet investigated getting detailed reports but this still seems like a regression. We're using a proper Maven project structure, so I guess a Sonar-Runner approach might treat modules differently and that's the source of the discrepancy. It would be nice to restore the simple reports for our use case.

Simply removing the overridden shouldExecuteOnProject method seems to result in the desired behaviour for our project, but I wouldn't submit a PR for this without fully understanding the original approach.

@jmecosta
Copy link
Member

I think some commit has been made later to fix some kind of corner case,
however the original idea was to support the visual studio bootstrapper
that creates modules. Indeed I don't think we have a maven test for this,
but feel free to create a PR to handle your use case. If it does not brake
the existent integration test it should be fine to merge.

The flag you mention I think it's to prevent every module to call all
sensors every time.

On Tue, Aug 18, 2015, 18:41 Peter van Zetten [email protected]
wrote:

We've realised (after not paying attention for a while) that we're not
getting test execution reports for our C++ tests. It's not a vital feature
of our SQ usage, but it used to work and I'm trying to narrow down why it
doesn't now.

I'm not sure of the intent behind f2cedf7
f2cedf7
and the general idea behind overriding shouldExecuteOnProject in the
CxxXunitSensor
https://github.com/wenns/sonar-cxx/blob/531e805204836f0fe955fcac14390b875c35db4e/sonar-cxx-plugin/src/main/java/org/sonar/plugins/cxx/xunit/CxxXunitSensor.java#L118.
The commit message from @jmecosta https://github.com/jmecosta
introducing the override just states "prevent simple mode from running in
all modules" and I can't figure out why that's desirable.

As it stands, we can't get simple test execution reports in our
multi-module project by any means. I haven't yet investigated getting
detailed reports but this still seems like a regression. We're using a
proper Maven project structure, so I guess a Sonar-Runner approach might
treat modules differently and that's the source of the discrepancy. It
would be nice to restore the simple reports for our use case.

Simply removing the overridden shouldExecuteOnProject method seems to
result in the desired behaviour for our project, but I wouldn't submit a PR
for this without fully understanding the original approach.


Reply to this email directly or view it on GitHub
#612.

@Zetten
Copy link
Contributor Author

Zetten commented Aug 19, 2015

After removing shouldExecuteOnProject (so it uses the default, which just checks for the existence of CXX files) the ITs still seem to run fine. The test reports for both googletest_project and boosttest_project are stored successfully.

I'll do a few more internal tests with a few other C/C++ projects, including a multi-module project using sonar-runner, and see if there's anything else that comes up. If it's clean I'll push for a PR.

@Zetten
Copy link
Contributor Author

Zetten commented Aug 25, 2015

Ok, everything seemed fine after some additional tests, so I created PR #617.

Please do check it out and try it locally though; I only tested in SQ 4.5.5 and with a few projects, so I'd be interested to see if it acts differently in any other configurations.

@guwirth
Copy link
Collaborator

guwirth commented Aug 27, 2015

Integration test for PR is failing.

@jmecosta
Copy link
Member

And it's failing on the multi module test, so you need it make sure that
test is passing

On Thu, Aug 27, 2015, 16:50 Günter Wirth [email protected] wrote:

Integration test for PR is failing.


Reply to this email directly or view it on GitHub
#612 (comment).

@guwirth guwirth added this to the M 0.9.4 milestone Sep 3, 2015
@guwirth guwirth removed this from the M 0.9.4 milestone Oct 19, 2015
@Bertk
Copy link
Contributor

Bertk commented Nov 5, 2015

Beside the fix for multi module project support the CxxXunitSensor should be improved to handle test results from multiple report files e.g. cppunit, gtest for a component as well.

@guwirth guwirth added this to the M 0.9.5 milestone Jan 16, 2016
@guwirth guwirth self-assigned this Jan 16, 2016
@guwirth
Copy link
Collaborator

guwirth commented Jan 16, 2016

please try again with 0.9.5:

@guwirth guwirth closed this as completed Jan 16, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

4 participants