Option for covering tests

[andre15silva]

This PR presents a new option for test-runner, covering test classes.

Closes #96
Related to ASSERT-KTH/flacoco#41

[coveralls]

Pull Request Test Coverage Report for Build 986463211

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

• For more information on this, see Tracking coverage changes with pull request builds.
• To avoid this issue with future PRs, see these Recommended CI Configurations.
• For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

• 25 of 38 (65.79%) changed or added relevant lines in 13 files are covered.
• 82 unchanged lines in 3 files lost coverage.
• Overall coverage increased (+0.2%) to 43.945%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/main/java/eu/stamp_project/testrunner/runner/coverage/JUnit5JacocoRunnerPerTestMethod.java	0	1	0.0%
src/main/java/eu/stamp_project/testrunner/listener/junit4/CoveragePerJUnit4TestMethod.java	2	4	50.0%
src/main/java/eu/stamp_project/testrunner/runner/coverage/JacocoRunner.java	8	10	80.0%
src/main/java/eu/stamp_project/testrunner/listener/junit4/CoveredTestResultsPerJUnit4TestMethod.java	0	4	0.0%
src/main/java/eu/stamp_project/testrunner/runner/ParserOptions.java	2	6	33.33%

Files with Coverage Reduction	New Missed Lines	%
src/main/java/eu/stamp_project/testrunner/listener/junit4/CoveragePerJUnit4TestMethod.java	3	94.12%
src/main/java/eu/stamp_project/testrunner/listener/junit4/CoveredTestResultsPerJUnit4TestMethod.java	19	25.45%
src/main/java/eu/stamp_project/testrunner/runner/coverage/JacocoRunner.java	60	42.76%

Totals
Change from base Build 968379322:	0.2%
Covered Lines:	1016
Relevant Lines:	2312

---

💛 - Coveralls

[andre15silva]

Btw, when using this in flacoco I get an issue only when the coverTests flag is true

Exception in thread "main" java.lang.RuntimeException: fr/spoonlabs/FLtest1/Calculator.class,/tmp/junit4526515788628245870/fr/spoonlabs/FLtest1/Calculator.class,fr.spoonlabs.FLtest1.Calculator
	at eu.stamp_project.testrunner.runner.coverage.JacocoRunner.instrumentAll(JacocoRunner.java:301)
	at eu.stamp_project.testrunner.runner.coverage.JacocoRunner.<init>(JacocoRunner.java:84)
	at eu.stamp_project.testrunner.runner.coverage.JacocoRunnerPerTestMethod.<init>(JacocoRunnerPerTestMethod.java:99)
	at eu.stamp_project.testrunner.runner.coverage.JacocoRunnerCoveredResultPerTestMethod.<init>(JacocoRunnerCoveredResultPerTestMethod.java:34)
	at eu.stamp_project.testrunner.runner.coverage.JUnit4JacocoRunnerCoveredResultPerTestMethod.<init>(JUnit4JacocoRunnerCoveredResultPerTestMethod.java:27)
	at eu.stamp_project.testrunner.runner.coverage.JUnit4JacocoRunnerCoveredResultPerTestMethod.main(JUnit4JacocoRunnerCoveredResultPerTestMethod.java:54)
Caused by: java.io.IOException: Error while instrumenting class fr.spoonlabs.FLtest1.Calculator.
	at org.jacoco.core.instr.Instrumenter.instrumentError(Instrumenter.java:166)
	at org.jacoco.core.instr.Instrumenter.instrument(Instrumenter.java:138)
	at eu.stamp_project.testrunner.runner.coverage.JacocoRunner.instrumentAll(JacocoRunner.java:299)
	... 5 more
Caused by: java.lang.IllegalArgumentException
	at org.jacoco.core.internal.Java9Support.readFully(Java9Support.java:45)
	at org.jacoco.core.instr.Instrumenter.instrument(Instrumenter.java:136)
	... 6 more

Any idea? It happens when instrumenting the class files for normal source code classes, which come from the same place as the normal tests.

[danglotb]

Hi @andre15silva

I'm sorry, but I have no clue what is happening here.

It happens when instrumenting the class files for normal source code classes, which come from the same place as the normal tests.

Could you explain a bit more? I'm sorry, I did not fully understand

[andre15silva]

Could you explain a bit more? I'm sorry, I did not fully understand

The last commit fixes the issue. The classloader in JacocoRunner wasn't being initialized according to the new input format.

Don't merge just yet tho, the coverage is including the tests but then doesn't register which instructions are ran. I'm trying to fix it, and will add new test cases to check that.

[andre15silva]

So, I think there is an issue with the way test-runner uses the JUnit listeners.

I have added two test cases that showcase the difference between the old way flacoco was using[1], and the new one through the EntryPoint[2].

[1] The old way of flacoco using test-runner is here https://github.com/SpoonLabs/flacoco/blob/4a99453ffc01305741b946049c3f7edef29e26b6/src/main/java/fr/spoonlabs/flacoco/core/coverage/CoverageRunner.java#L42-L72.

The corresponding function in JacocoRunner is

test-runner/src/main/java/eu/stamp_project/testrunner/runner/coverage/JacocoRunner.java

Lines 233 to 285 in d7dd726

	/**
	* We indicate the method to compute the coverage
	* @param parentClassloader
	* @return
	*/
	public CoveredTestResult run(CoverageTransformer coverageCollector, ClassLoader parentClassloader, String classesDirectory, String testClassesDirectory,
	String fullQualifiedNameOfTestClass, boolean coverTest, String... testMethodNames) {
	final RuntimeData data = new RuntimeData();
	final ExecutionDataStore executionData = new ExecutionDataStore();
	final SessionInfoStore sessionInfos = new SessionInfoStore();
	try {
	runtime.startup(data);
	//TODO: I dont understand why to re-generate the class loader each time we execute the test. If I remove it, it does not compute the coverage from the second execution
	String classesToInstrument =classesDirectory +File.pathSeparator+ testClassesDirectory ;
	this.recreateInstrumentedClassloaded(parentClassloader, classesToInstrument,
	this.instrumentedClassLoader.getDefinitions());
	final CoveredTestResult listener = executeTest(new String[] { fullQualifiedNameOfTestClass }, testMethodNames,
	Collections.emptyList());
	if (!listener.getFailingTests().isEmpty()) {
	System.err.println("Some test(s) failed during computation of coverage:\n" + ((TestResult) listener)
	.getFailingTests().stream().map(Failure::toString).collect(Collectors.joining("\n")));
	}
	data.collect(executionData, sessionInfos, false);
	runtime.shutdown();
	clearCache(this.instrumentedClassLoader);
	String pathToCover = (coverTest)? classesToInstrument: classesDirectory;
	Coverage coverageSource = coverageCollector.transformJacocoObject(executionData, pathToCover);
	listener.setCoverageInformation(coverageSource);
	return listener;
	} catch (
	Exception e) {
	System.out.println("Error: could not collect test data");
	e.printStackTrace();
	return null;
	}
	}

.

[2] The new way of using test-runner, through the EntryPoint means that we have a listener (a RunListener in the case of JUnit4), which does the initialization and collection previously done in the run method of JacocoRunner when the tests start and finish

test-runner/src/main/java/eu/stamp_project/testrunner/listener/junit4/CoveredTestResultsPerJUnit4TestMethod.java

Lines 63 to 92 in d7dd726

	@Override
	public synchronized void testStarted(Description description) throws Exception {
	this.internalCoveredTestResult.setExecutionData(new ExecutionDataStore());
	this.internalCoveredTestResult.setSessionInfos(new SessionInfoStore());
	this.internalCoveredTestResult.getData().setSessionId(description.getMethodName());
	this.internalCoveredTestResult.getData().collect(
	this.internalCoveredTestResult.getExecutionData(),
	this.internalCoveredTestResult.getSessionInfos(),
	true
	);
	}
	@Override
	public synchronized void testFinished(Description description) throws Exception {
	this.internalCoveredTestResult.getRunningTests().add(description.getMethodName());
	this.internalCoveredTestResult.getData().collect(
	this.internalCoveredTestResult.getExecutionData(),
	this.internalCoveredTestResult.getSessionInfos(),
	false
	);
	Coverage jUnit4Coverage =
	internalCoveredTestResult.getCoverageTransformer().transformJacocoObject(this.internalCoveredTestResult.getExecutionData(),
	this.internalCoveredTestResult.getClassesDirectory());
	this.internalCoveredTestResult.getCoverageResultsMap().put(description.getMethodName(), jUnit4Coverage);
	if (isParametrized.test(description.getMethodName())) {
	this.collectForParametrizedTest(fromParametrizedToSimpleName.apply(description.getMethodName()));
	}
	}

So, after debugging, my current hypothesis is that the issue is related to this difference, with the test statements somehow not being recorded until the whole execution finishes?
I have modifying this, but didn't get anywhere. I have searched for similar issues, but haven't found any.

@danglotb @martinezmatias do any of you have any idea what could be causing this?

[danglotb]

I did not completely understand the issue but one thing is certain:

You should not use directly the JacocoRunner, as it would result in a conflict of classloading, but rather use the API provided by EntryPoint, which will run a java command line, and therefore starts a new JVM, avoiding any classloading problems.

[andre15silva]

You should not use directly the JacocoRunner, as it would result in a conflict of classloading, but rather use the API provided by EntryPoint, which will run a java command line, and therefore starts a new JVM, avoiding any classloading problems.

Yes, definitely. That's why we already moved to using EntryPoint (see ASSERT-KTH/flacoco#35)

I did not completely understand the issue

So, one feature that was broken when we started using EntryPoint was the coverage of test classes being computed, which this PR is meant to fix. The instrumentation works, but the returned results are all zeros.

For example:

• Obtained through JacocoRunner: example/TestSuiteExample=Lines covered [init=2, end=45, cov={2=3, 3=0, 5=0, 6=0, 7=0, 8=0, 9=0, 12=0, 13=0, 14=0, 15=0, 16=0, 19=0, 20=0, 21=0, 22=0, 25=4, 26=7, 27=1, 28=0, 31=0, 32=0, 33=0, 34=0, 37=0, 38=0, 39=0, 40=0, 43=0, 44=0, 45=0, 46=0}]
• Obtained through EntryPoint: example/TestSuiteExample=Lines covered [init=2, end=45, cov={2=0, 3=0, 5=0, 6=0, 7=0, 8=0, 9=0, 12=0, 13=0, 14=0, 15=0, 16=0, 19=0, 20=0, 21=0, 22=0, 25=0, 26=0, 27=0, 28=0, 31=0, 32=0, 33=0, 34=0, 37=0, 38=0, 39=0, 40=0, 43=0, 44=0, 45=0, 46=0}]

My hypothesis is that this occurs due to the data collection having moved from the run method we were using in JacocoRunner to the JUnit listeners. Is it possible that the instructions of test classes are only registered after the listener method testFinished has been called?

[danglotb]

I'm sorry I cannot see it through right now but I remember having such troubles by the time I was developing test-runner.

I fetch your branch and ran eu.stamp_project.testrunner.runner.coverage.JacocoRunnerTest#testRunCoveredTestResults and obtained:

java.lang.AssertionError: 
Expected :Optional[2]
Actual   :3

I'm sorry but I would like to have a complete scenario that allows me to reproduce. Could you provide me this or help me to find it?

[andre15silva]

I'm so sorry, I seem to have reverted a change before committing 🤦 . Assertions are working now.

The two tests in question are EntryPointTest#testRunCoveredTestResultPerTestMethodsDetailedCoverageCoverTests and JacocoRunnerTest#testRunCoveredTestResults.

The first one shows the buggy behavior, since the expected result is the one the second one returns. I added some prints in the test cases to show the two versions that should be equal.

This means that the coverage of class example.TestSuiteExample, of which method test8 is ran, should not be full of 0s, but have some execution going through it.

I think these two tests provide the complete scenario, with the expected behavior from the previous version, and the buggy behavior from the new version, but let me know if you need anything else! Thank you for your time :)

[danglotb]

Hi @andre15silva

Is this still a work in progress or should we close it?

[andre15silva]

Is this still a work in progress or should we close it?

Hi @danglotb. I'd say we close it, I'm no longer working on it.