Support #10113
evaluate and test code coverage tooling
100%
History
#1 Updated by Greg Shah about 1 year ago
The idea is to use existing tooling such as JaCoCo or JCov to calculate code coverage for an area of FWD code. For now, we can test it for the Lexer tests written in #6860. Some requirements:
- open source
- allows command line usage (does not require IDE for usage)
- can be disabled completely leaving no runtime traces
- we prefer build-time implementation rather than runtime
Please note that some time ago, a customer was using JaCoCo for this purpose and there was a bug reported in FWD which could only be recreated if JaCoCo was present. In other words, the runtime tricks JaCoCo was playing was breaking FWD.
#2 Updated by Florin Eugen Rotaru about 1 year ago
- Status changed from New to WIP
- reviewer Roger Borrello added
Using JaCoCo, there are two ways to monitor the code coverage.
- First is wrapping the ant task which runs the Java into a
jacoco:coveragetask:<jacoco:coverage> <java classname="org.jacoco.examples.HelloJaCoCo" fork="true"> <classpath> <pathelement location="./bin"/> </classpath> </java> </jacoco:coverage>
- Or, an argument can be passed in form of java agent, which has the same effect.
For adding conversion code coverage tracking, we can opt for the second approach, so we won't have to rewrite the ant targets.
#3 Updated by Florin Eugen Rotaru about 1 year ago
- Status changed from WIP to Review
- % Done changed from 0 to 80
I have managed to add code coverage measuring for the ant convert tag (I did it for the Dataset project).
Firstly, in the p2j branch I added this build.gradle dependencies:
dependencies {
...
fwdClientServer group: 'org.jacoco', name: 'org.jacoco.ant', version: '0.8.13'
...
}
So after running ant deploy.prepare, the needed jars will be available in the project's deploy/lib directory.
Then, I added these lines to the Dataset's build.xml file:
<project name="dataset" default="dist" basedir="." xmlns:if="ant:if"> ... <property name="jacoco.exec" value="${basedir}/coverage/jacoco.exec"/> <taskdef name="jacocoagent" classname="org.jacoco.ant.AgentTask"> <classpath> <pathelement location="${deploy.home}/lib/org.jacoco.ant-0.8.13.jar"/> <pathelement location="${deploy.home}/lib/org.jacoco.core-0.8.13.jar"/> <pathelement location="${deploy.home}/lib/org.jacoco.agent-0.8.13.jar"/> </classpath> </taskdef> <taskdef name="jacocoreport" classname="org.jacoco.ant.ReportTask"> <classpath> <pathelement location="${deploy.home}/lib/org.jacoco.ant-0.8.13.jar"/> <pathelement location="${deploy.home}/lib/org.jacoco.core-0.8.13.jar"/> <pathelement location="${deploy.home}/lib/org.jacoco.report-0.8.13.jar"/> <fileset dir="${deploy.home}/lib"> <include name="asm*9.8.jar"/> </fileset> </classpath> </taskdef> ... <jacocoagent property="agentvmparam" destfile="${jacoco.exec}"/> <!-- convert Progress code from ${app.4gl.src} into ${src}, using ${app.4gl.pattern} pattern --> <target name="convert" depends="init, check-file-cvt-list" description="Convert the Progress ABL source code to Java source code, using a pattern as input." unless="file-cvt-list"> <record name="cvt_${LOG_STAMP}.log" action="start"/> <java classname="com.goldencode.p2j.convert.ConversionDriver" fork="true" failonerror="true" dir="${basedir}"> <jvmarg if:set="coverage" value="${agentvmparam}"/> <!---- only added this optional argument--> ... </java> <record name="cvt_${LOG_STAMP}.log" action="stop"/> </target> <!-- This generates the report, using the .exec, the classes and the source code--> <target name="coverage-report"> <jacocoreport> <executiondata> <file file="${jacoco.exec}"/> </executiondata> <structure name="JaCoCo Report"> <classfiles> <fileset dir="${fwd.build.home}/classes/com/goldencode/p2j"> <include name="**/*.class"/> </fileset> </classfiles> <sourcefiles encoding="UTF-8"> <fileset dir="${fwd.src.home}"> <include name="**/*.java"/> </fileset> </sourcefiles> </structure> <html destdir="coverage"/> </jacocoreport> </target> ... </project>
Running ant convert -Dcoverage=yes and then ant coverate-report will generate the reports in the coverate directory.
Roger/Greg: Please review and let me know what the next step should be. I created a branch 10113a to add the Gradle dependency.
#4 Updated by Florin Eugen Rotaru about 1 year ago
Added Roger.
#5 Updated by Roger Borrello about 1 year ago
That looks like a great addition. Is there any significant time added to the build? Can you post a sample report?
#6 Updated by Florin Eugen Rotaru about 1 year ago
- File coverage.patch
added - File coverage.zip added
Roger Borrello wrote:
That looks like a great addition. Is there any significant time added to the build? Can you post a sample report?
I have tested with the Hotel conversion. Usually the conversion takes 3m 30s. With the jacoco agent it increases to 4m 30s. I have attached a report sample (see coverage.zip some packages are removed since the archive was exceeding redmine's size limit).
I have also added the build.xml patch wrt the last revision. Running and deploy.prepare and then ant convert.zset -Dcoverage=yes will do the conversion and generate the report.
#7 Updated by Roger Borrello about 1 year ago
I can't say as I am familiar with what JaCoCo is doing, but it looks great!
It still seems a little intrusive to the project's build.xml. Is there a "standalone" build-coverage.xml that could be created that wouldn't need to be injected into the project's build.xml? So you would just run ant -f build-coverage.xml?
#8 Updated by Florin Eugen Rotaru about 1 year ago
Roger Borrello wrote:
I can't say as I am familiar with what JaCoCo is doing, but it looks great!
It still seems a little intrusive to the project's
build.xml. Is there a "standalone"build-coverage.xmlthat could be created that wouldn't need to be injected into the project'sbuild.xml? So you would just runant -f build-coverage.xml?
That's a good idea, I will try it out.
#9 Updated by Florin Eugen Rotaru about 1 year ago
- % Done changed from 80 to 100
- File build_coverage.xml
added
I have moved the Ant code in a separate build_coverage.xml. I have attached the file. Running ant -f build_coverage.xml does the conversion and generates the reports in the "coverage" directory.
#10 Updated by Roger Borrello about 1 year ago
Florin Eugen Rotaru wrote:
Excellent... very compact. Keeping your newI have moved the Ant code in a separate
build_coverage.xml. I have attached the file. Runningant -f build_coverage.xmldoes the conversion and generates the reports in the "coverage" directory.
build_coverage.xml in mind, please analyze how it might change from project-to-project, such as with Hotel GUI, and the testcases, and possibly a couple of customer applications. Let's determine:
- Can it be generalized to work for all (or almost all) of the applications we might see without any (or just minor) modifications?
- If there are minor modifications, can they be extracted out with configurable properties?
The requirement would be that if we'd need to evaluate, we'd just need to drop the xml and perhaps a properties file, and let it rip, then gather the report. Speaking of which, there should be an "archive" task that creates a zip file of the coverage report.
#11 Updated by Roger Borrello about 1 year ago
Roger Borrello wrote:
- Can it be generalized to work for all (or almost all) of the applications we might see without any (or just minor) modifications?
I think the main thing is understanding how the application gathers the code, and being able to handle it. This is typically the conversion list (z-file) or a profile, or just everything. We'd need to cover all the same ways and using the app4gl.pattern from build.properties
#12 Updated by Greg Shah about 1 year ago
With #9725 we are getting rid of the old ways of running conversion, so we should not spend time on those.
#13 Updated by Florin Eugen Rotaru about 1 year ago
- File build_coverage.xml
added
Greg Shah wrote:
With #9725 we are getting rid of the old ways of running conversion, so we should not spend time on those.
Then I guess we should stick to the zset-file based conversion. I have updated the build_coverage.xml and added a zip target.
Roger Borrello wrote:
If there are minor modifications, can they be extracted out with configurable properties?
One difference that I noticed in a customer's (GUI) build.xml is the use of additional <classpath refid .... with some element when running the ConversionDriver, probably to make use of some additional java dependencies.
#14 Updated by Florin Eugen Rotaru about 1 year ago
I suppose that can be solved if for each such case the classpath passed to the ConversionDriver is adapted to contain all neded jars, such as:
<path id="compile.classpath">
<fileset dir="${p2jlib.home}">
<include name="*.jar"/>
</fileset>
</path>
<path id="app.convert.classpath">
<fileset dir="${build.home}/lib">
<include name="{client_specific}.ext.jar"/>
</fileset>
</path>
<path id="result.classpath">
<pathelement pathref="compile.classpath"/>
<pathelement pathref="app.convert.classpath"/>
</path>
And then pass this result.classpath to the Driver.
#15 Updated by Florin Eugen Rotaru about 1 year ago
- File build_coverage_testcases.xml
added
I've attached a build_coverage.xml for the testcases project. It's based on a zfile_set.xml. It needs a p2j which has the Jacoco binaries and will manually copy them to the project's deploy/lib. In other words just running ant -f build_coverage.xml is enough for running the conversion. I have committed it to the repo.
#16 Updated by Florin Eugen Rotaru about 1 year ago
Should I devise such a script for all the remaining projects and post them here for review?
#17 Updated by Roger Borrello about 1 year ago
Florin Eugen Rotaru wrote:
IIRC, the differences between the classpaths that are setup within theRoger Borrello wrote:
If there are minor modifications, can they be extracted out with configurable properties?
One difference that I noticed in a customer's (GUI)
build.xmlis the use of additional<classpath refid ....with some element when running theConversionDriver, probably to make use of some additional java dependencies.
build.xml were to try to minimize the length of the particular classpath that is used for:
- Conversion of the application
- Compilation of the application, which may require additional libraries
- Creation/import of the DB (usually through
build_db.xmlusage)
because on some platforms, like Windows, if the classpath is too long,antbroke. That is my recollection.
Should the JaCoCo tool need the same classpath as the conversion or compilation?
#18 Updated by Greg Shah about 1 year ago
- Status changed from Review to Internal Test
Overall, I'm OK with the approach. I like that it leaves the existing ant rules unchanged. The primary drawback is that we will have to maintain the convert.zset target when we edit the matching rules in build.xml. Invariably, the build_coverage.xml will fall behind and need edits before running. We can live with that.
Should I devise such a script for all the remaining projects and post them here for review?
Not right now. It seems like we can just copy this when needed. I don't want to have to maintain this in every project. We should have this in Hotel and in Testcases, that should be enough.
#19 Updated by Florin Eugen Rotaru about 1 year ago
Shall the change to the p2j mentioned in #10113-3 be merged?
#20 Updated by Greg Shah about 1 year ago
Shall the change to the
p2jmentioned in #10113-3 be merged?
I don't want JaCoCo to be a standard dependency of FWD. We only need it in the very rare case that we are evaluating code coverage. How can we handle that outside of the FWD build?
#21 Updated by Florin Eugen Rotaru about 1 year ago
Greg Shah wrote:
Shall the change to the
p2jmentioned in #10113-3 be merged?I don't want JaCoCo to be a standard dependency of FWD. We only need it in the very rare case that we are evaluating code coverage. How can we handle that outside of the FWD build?
Maybe by adding the jacoco jars in a dedicated directory in the project's ./deploy directory? And the build_coverage.xml will take search for them there.
#22 Updated by Greg Shah about 1 year ago
How about adding code to build_coverage.xml to download and copy the jars if they are not there? That way the dependency is not in FWD itself, and it is contained in the same rules that are used to run the coverage testing.
#23 Updated by Florin Eugen Rotaru about 1 year ago
Greg Shah wrote:
How about adding code to
build_coverage.xmlto download and copy the jars if they are not there? That way the dependency is not in FWD itself, and it is contained in the same rules that are used to run the coverage testing.
OK, looking into it.
#24 Updated by Florin Eugen Rotaru about 1 year ago
- File build_coverage.xml
added
I have committed a the new build_coverage.xml to Testcases. It now downloads the dependencies(only if they are missing). We've also added code coverage reporting for the Preprocessor to cover Stefan's tests. We now have the preproc-tests and convert-tests targets.
I am also attaching the build_coverage.xml meant for HOTEL GUI.
#25 Updated by Florin Eugen Rotaru about 1 year ago
- Status changed from Internal Test to Pending
I have ran the conversion with one big GUI customer application. It took a bit longer (Maybe 20%) but it finished OK. We are waiting for Stefan's 6859b branch to be merged, then we have to slightly modify the build_coverage.xml and then we can proceed with a commit to Hotel GUI.