Option for covering tests #98

andre15silva · 2021-06-30T13:19:08Z

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

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

Signed-off-by: André Silva <[email protected]>

coveralls · 2021-06-30T13:21:11Z

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

...u/stamp_project/testrunner/runner/coverage/JUnit4JacocoRunnerCoveredResultPerTestMethod.java

Signed-off-by: André Silva <[email protected]>

andre15silva · 2021-06-30T13:26:55Z

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 · 2021-06-30T13:33:32Z

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

Signed-off-by: André Silva <[email protected]>

andre15silva · 2021-06-30T14:23:44Z

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.

Signed-off-by: André Silva <[email protected]>

andre15silva · 2021-07-01T11:42:22Z

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 · 2021-07-01T12:01:39Z

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 · 2021-07-01T12:11:24Z

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 · 2021-07-01T13:51:04Z

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?

Signed-off-by: André Silva <[email protected]>

andre15silva · 2021-07-01T14:13:39Z

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 · 2022-05-02T06:45:33Z

Hi @andre15silva

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

andre15silva · 2022-05-02T09:39:09Z

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.

Implement coverTests option

a69d67f

Signed-off-by: André Silva <[email protected]>

danglotb reviewed Jun 30, 2021

View reviewed changes

...u/stamp_project/testrunner/runner/coverage/JUnit4JacocoRunnerCoveredResultPerTestMethod.java Outdated Show resolved Hide resolved

danglotb reviewed Jun 30, 2021

View reviewed changes

...u/stamp_project/testrunner/runner/coverage/JUnit4JacocoRunnerCoveredResultPerTestMethod.java Outdated Show resolved Hide resolved

Remove debug prints

7881def

Signed-off-by: André Silva <[email protected]>

danglotb approved these changes Jun 30, 2021

View reviewed changes

Fix classloader initialization

0af35b5

Signed-off-by: André Silva <[email protected]>

Test test class coverage

e879426

Signed-off-by: André Silva <[email protected]>

Fix assertions

bf9fc05

Signed-off-by: André Silva <[email protected]>

andre15silva mentioned this pull request Jul 6, 2021

Expand the usage of fully qualified method names for all the API #102

Merged

danglotb force-pushed the master branch from de4c578 to 089922b Compare February 10, 2022 14:59

andre15silva closed this May 2, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Option for covering tests #98

Option for covering tests #98

andre15silva commented Jun 30, 2021 •

edited

Loading

coveralls commented Jun 30, 2021 •

edited

Loading

andre15silva commented Jun 30, 2021 •

edited

Loading

danglotb commented Jun 30, 2021 •

edited

Loading

andre15silva commented Jun 30, 2021 •

edited

Loading

andre15silva commented Jul 1, 2021

danglotb commented Jul 1, 2021

andre15silva commented Jul 1, 2021

danglotb commented Jul 1, 2021

andre15silva commented Jul 1, 2021

danglotb commented May 2, 2022

andre15silva commented May 2, 2022

Option for covering tests #98

Option for covering tests #98

Conversation

andre15silva commented Jun 30, 2021 • edited Loading

coveralls commented Jun 30, 2021 • edited Loading

Pull Request Test Coverage Report for Build 986463211

Warning: This coverage report may be inaccurate.

Details

💛 - Coveralls

andre15silva commented Jun 30, 2021 • edited Loading

danglotb commented Jun 30, 2021 • edited Loading

andre15silva commented Jun 30, 2021 • edited Loading

andre15silva commented Jul 1, 2021

danglotb commented Jul 1, 2021

andre15silva commented Jul 1, 2021

danglotb commented Jul 1, 2021

andre15silva commented Jul 1, 2021

danglotb commented May 2, 2022

andre15silva commented May 2, 2022

andre15silva commented Jun 30, 2021 •

edited

Loading

coveralls commented Jun 30, 2021 •

edited

Loading

andre15silva commented Jun 30, 2021 •

edited

Loading

danglotb commented Jun 30, 2021 •

edited

Loading

andre15silva commented Jun 30, 2021 •

edited

Loading