Details
-
Type:
Bug
-
Status:
Open
-
Priority:
Critical
-
Resolution: Unresolved
-
Affects Version/s: 2.2
-
Fix Version/s: None
-
Labels:None
-
Number of attachments :3
Description
I want run as part of my CI process the goals:
mvn install cobertura:cobertura
The problem is that this causes the test phase to be ran twice.
AFAIK, there's no way to do this and it's a major issue when doing CI.
-
- patch
- 21/Oct/09 2:10 PM
- 3 kB
- Ryan Stewart
-
- report-only.patch
- 11/Jun/10 1:12 PM
- 2 kB
- Charles Honton
-
- reportOnlyAndInstrumentOutputDirPatch.patch
- 01/Feb/12 5:51 PM
- 4 kB
- Shant Stepanian
Activity
Surefire report plugin has a goal which creates reports only without running tests. Cobertura plugin should be able to do the same.
Be aware, running the tests twice is actually a good thing.
I have had tests that pass when run with cobertura even though the underlying code is broken... i.e. running the tests normally and they fail!
I have had tests that fail when run with cobertura and pass without...
In both cases there is usually a bug that I would not have found if I had only run the tests once.
Please reconsider trying to run them only once.... in reality you should try and run them five or six times (Sun JVM, IBM JVM, BEA JVM, both with and without coverage... and consider using both cobertura and emma)
If there is a discrepancy between any of these runs then likely you have badly written code and a bug just waiting to be found.
Stephen, i agree that tests ideally should be run several times in different conditions. But let's not stray from this issue.
The problem at hands is that in this use case cobertura forces the exact same tests under the same conditions to be run twice, this both causes heavier load and also erratic reports in some CI servers as Rafael as pointed out.
I'm actually run the tests 3 times now. One for surefire, another for cobertura to fail the build if the coverage isn't met and another for the cobertura report.
Given Stephen's comment, running tests twice is a feature? Even though there might be good reasons to pass tests twice I think users should have the freedom of overriding this default behavior and run tests only once.
I doubt that every application are writing tests that tests thread sync.
While most application tests execution takes at least a few minutes.
It sounds to me that systematically doubling the execution time just because there might be a test that cobertura will mess up, is kinda overkill.
Make it an option to run the tests twice (for example by setting to executions of surefire at different times).
Make it that I can run in a few seconds my unit tests, verify coverage of the classes I modified, and let me run a way longer nightly build where I can run integration-tests as well as instrumented/non-intrumented validations.
You might add a large warning in cobertura plugin documentations of all the consequences of using coverage upon the reliance of our tests.
I'm attaching a patch file with a new class, CoberturaReportOnlyMojo, that mostly duplicates code from the CoberturaReportMojo and allows you to run a cobertura report without triggering the test phase in a parallel lifecycle. It implements a new goal named "report". To use it in its current form, do three things:
1) Configure the plugin to specify output formats and bind the instrument goal to the process-test-classes phase, like so:
<plugin> <groupId>org.codehaus.mojo</groupId> <artifactId>cobertura-maven-plugin</artifactId> <configuration> <formats> <format>xml</format> <format>html</format> </formats> </configuration> <executions> <execution> <id>cobertura-instrument</id> <phase>process-test-classes</phase> <goals> <goal>instrument</goal> </goals> </execution> </executions> </plugin>
2) Modify the surefire plugin to point to the instrumented classes instead of the default target/classes. By default, the plugin places the instrumented classes at target/generated-classes/cobertura:
<plugin> <artifactId>maven-surefire-plugin</artifactId> <configuration> <classesDirectory>${project.build.directory}/generated-classes/cobertura</classesDirectory> </configuration> </plugin>
3) Run "mvn clean test cobertura:report", and your reports will, by default, be in $
{project.build.directory}/cobertura. Naturally, you could bind the report goal to a lifecycle phase if you wanted to.
So, is there any news about if this is going to be fixed any time soon, especially that it's one of the really popular issues?
Surefire had similar problem and was fixed with a Report-only mojo
The patch above may make cobertura:report behave.. but cobertura:verify will still trigger a new build. So helpful, not not perfect for our use case...
Any news on this?
BTW http://nayidisha.com/techblog/an-over-eager-code-coverage-tool also links to a patch which is reported to work fine.
Hi,
Is there an update on this? We ran into this issue as well.
I have attached a patch (cobertura-report-only-patch-20120201.patch) that defines a "report" goal that just generates the report and does nothing else. The existing functionality remains.
This seems like something reasonable to add, as we already have a corresponding "instrument" task, so why not a standalone report task?
Is it possible to get this into the next release?
I'd be willing to coordinate w/ someone on this to get this in
Thanks,
Shant
Let me just quote Stephen again:
Be aware, running the tests twice is actually a good thing.
I have had tests that pass when run with cobertura even though the underlying code is broken... i.e. running the tests normally and they fail!
I have had tests that fail when run with cobertura and pass without...
In both cases there is usually a bug that I would not have found if I had only run the tests once.
Please reconsider trying to run them only once.... in reality you should try and run them five or six times (Sun JVM, IBM JVM, BEA JVM, both with and without coverage... and consider using both cobertura and emma)
If there is a discrepancy between any of these runs then likely you have badly written code and a bug just waiting to be found.
Maybe it should be surefire which should detect that it is actually running exactly the same test-run again...
Be aware, running the tests twice is actually a good thing.
Others have said this before and I agree: yes, running tests under different conditions is a maybe a good thing, but certainly no tool should force this behaviour upon me.
E.g. It would also be a good idea to run tests multiple times in different run orders to prevent inadverted dependencies between tests. Still I wouldn't want surefire to do this when I wouldn't request it to do so.
Agreed w/ Christoph on this - various clients may have different use cases, and so the plugin should not force this behavior on everyone. There are many requests on this JIRA ticket and others (not to mention that this JIRA now references patches from 4 people who tried to solve this issue), so it seems like it should be addressed in the next version and it would benefit a lot of people.
In addition, I noticed that the "instrument" goal was changing the build output directory to the instrumented directory. This had negative consequences on our project as the jar plugin was now packaging the instrumented files. I've replaced my previous patch w/ a new one that configures the output directory change as a configuration parameter.
In general, it seems like there should be two usages of this plugin:
- via the cobertura goal, which essentially does everything for you
- via specific usage of the instrument and "report" goals, which allows finer grained control, including the ability to not run your tests twice. The last patch I had on not changing the outputDirectory goes along with this
Please let us know when these changes can be added. I'd be willing to work w/ someone on this
Thanks,
Shant
If the tests are run on two different folders, namely on target/classes (the original sources) and on target/generated-sources/cobertura (the instrumented sources) then that's the expected behavior. If we change this and something goes wrong everybody will point at us.
If the instrumented classes end up in the jar, then that's a bug.
Exactly the same test-runs shouldn't run twice since SUREFIRE-257
What about making this change in behavior configurable, but not the default option? Then you don't have to worry about finger pointing, but still give people the ability to speed up their builds if they are willing to accept a mild increase in risk.
See the attributes on http://mojo.codehaus.org/cobertura-maven-plugin/cobertura-mojo.html
- Invokes the execution of the lifecycle phase test prior to executing itself.
- Executes in its own lifecycle: cobertura.
You can't change attributes like you can with parameters, so optional is not an option.
Possibly the intent of my fix was not clear earlier, given the subject-line of this JIRA.
What I'd like to propose (and this is basically what Ryan Stewart had proposed earlier in this JIRA) is to leave the cobertura goal alone, and instead add support for a report-only goal. A similar "report-only" goal looks like was proposed in the SUREFIRE-257 jira that you referenced for surefire
What I think people are wanting is just a way to not have the tests execute twice. It seems to be difficult to do that via the cobertura task or via some configuration in the surefire plugin. So what we'd like is just a basic report-only goal, which is already available in many other similar plugins.
Hence, if people wanted to just have run the tests once, they would access individual goals, similar to Ryan Stewart's proposal, i.e. 1) call the instrument goal 2) have the surefire plugin point to the instrumented folder 3) call the report-only goal to generate the report.
The reportOnlyAndInstrumentOutputDirPatch.patch has a patch for the report-only goal, as well as a configuration in the instrument goal to not switch the output folder, as otherwise having access to the instrument goal itself causes more trouble than we needed (as only the surefire plugin really needs the instrumented classes - all other plugins like code analysis [say findbugs] and jar/packaging need the actual classes).
This should be a reasonable compromise where 1) all behaviors for existing goals and configurations remain as is, including the cobertura task 2) people can still have better access to specific goals to handle executions the way that they want, if they choose that route. Short of having a better integration between surefire and cobertura, this seems like a best way to go, and would solve problems
Another option is to explicitly embed skipTests=false in the lifecycle.xml file, as mentioned at this link
http://nayidisha.com/techblog/an-over-eager-code-coverage-tool Thus, developers can configure their surefire plugin as they want, and add <skipTests>true</skipTests> in their pom. Thus, when the regular test execution goes through, it will get skipped; but it will get fired on the cobertura plugin. From seeing the feedback on that link, it seems like it had success for people
Either approach would seem fine, whether improving the standalone goals to work as described in my previous post, or to embed some property in the cobertura lifecycle that can toggle test execution in that lifecycle while disabling it in the regular execution. If you are uncomfortable w/ the variable being called "skipTests", then possibly you can coordinate w/ the surefire folks to see if they can add some variable like "runOnlyForCoverage" tests, which could be useful for other code coverage plugins too like EMMA or Clover
I have the same issue but with running mvn install site. It is a real pain as our CI server teamcity will show that we have run twice as many tests and display duplicate entries of all the tests run. It also take the build longer to run, and gets worse with more tests we write.
From my investigation into solving this issue some people prefer to run the tests twice as instrumentation can cause issues with run certain types of tests. So maybe a config flag would be nice?