diff --git a/test2code/src/main/kotlin/com/epam/drill/agent/test2code/Test2Code.kt b/test2code/src/main/kotlin/com/epam/drill/agent/test2code/Test2Code.kt index 8adcf4fa..eda52523 100644 --- a/test2code/src/main/kotlin/com/epam/drill/agent/test2code/Test2Code.kt +++ b/test2code/src/main/kotlin/com/epam/drill/agent/test2code/Test2Code.kt @@ -67,7 +67,7 @@ class Test2Code( sender = sender, collectReleasedProbes = { coverageManager.pollRecorded() }, collectUnreleasedProbes = { coverageManager.getUnreleased() }, - classProbePositions = coverageManager.classProbePositions + classMethodsMetadata = coverageManager.classMethodsMetadata ) private val coverageCollectionEnabled = configuration.parameters[COVERAGE_COLLECTION_ENABLED] private val classScanningEnabled = configuration.parameters[Test2CodeParameterDefinitions.CLASS_SCANNING_ENABLED] diff --git a/test2code/src/main/kotlin/com/epam/drill/agent/test2code/classparsing/Ast.kt b/test2code/src/main/kotlin/com/epam/drill/agent/test2code/classparsing/Ast.kt index b76f35b6..b8988121 100644 --- a/test2code/src/main/kotlin/com/epam/drill/agent/test2code/classparsing/Ast.kt +++ b/test2code/src/main/kotlin/com/epam/drill/agent/test2code/classparsing/Ast.kt @@ -131,9 +131,7 @@ fun parseAstClass(className: String, classBytes: ByteArray): List { } } -private fun AstMethod.classSignature() = - "${name}/${params}/${returnType}" - +private fun AstMethod.classSignature() = "${classname}:${name}:${params}:${returnType}" private fun getReturnType(methodNode: MethodNode): String { val returnTypeDesc: String = Type.getReturnType(methodNode.desc).descriptor diff --git a/test2code/src/main/kotlin/com/epam/drill/agent/test2code/classparsing/Checksum.kt b/test2code/src/main/kotlin/com/epam/drill/agent/test2code/classparsing/Checksum.kt index f8375874..e182fc9f 100644 --- a/test2code/src/main/kotlin/com/epam/drill/agent/test2code/classparsing/Checksum.kt +++ b/test2code/src/main/kotlin/com/epam/drill/agent/test2code/classparsing/Checksum.kt @@ -24,7 +24,7 @@ import java.io.ByteArrayInputStream val logger = KotlinLogging.logger { } -internal fun calculateMethodsChecksums( +fun calculateMethodsChecksums( classBytes: ByteArray, className: String ): Map = ClassParser(ByteArrayInputStream(classBytes), className) @@ -32,12 +32,11 @@ internal fun calculateMethodsChecksums( .methods // Filter needed for skipping interfaces, which have no opcodes for calculating checksum .filter { it.code != null } - .map { method -> method.classSignature() to calculateChecksum(method, className) } + .map { method -> method.classSignature(className) to calculateChecksum(method, className) } .filter { it.second != "" } .associate { it.first to it.second } -fun Method.classSignature() = - "${name}/${argumentTypes.asSequence().map { type -> type.toString() }.joinToString(separator = ",")}/${returnType}" +fun Method.classSignature(className: String) = "${className}:${name}:${argumentTypes.asSequence().map { type -> type.toString() }.joinToString(separator = ",")}:${returnType}" private fun calculateChecksum( method: Method, diff --git a/test2code/src/main/kotlin/com/epam/drill/agent/test2code/coverage/CoverageManager.kt b/test2code/src/main/kotlin/com/epam/drill/agent/test2code/coverage/CoverageManager.kt index cb3adffa..e6d319e8 100644 --- a/test2code/src/main/kotlin/com/epam/drill/agent/test2code/coverage/CoverageManager.kt +++ b/test2code/src/main/kotlin/com/epam/drill/agent/test2code/coverage/CoverageManager.kt @@ -24,7 +24,8 @@ open class CoverageManager( ) : IProbesProxy, ICoverageRecorder by threadCoverageRecorder { - val classProbePositions: ConcurrentHashMap>> = ConcurrentHashMap() + // TODO doesn't make much sense to store it here, if we use it only in coverage sender + val classMethodsMetadata: ConcurrentHashMap = ConcurrentHashMap() override fun invoke( id: Long, @@ -39,14 +40,14 @@ open class CoverageManager( id = id, probes = AgentProbes(probeCount), sessionId = coverage.context.sessionId, - testId = coverage.context.testId, + testId = coverage.context.testId ) } return execDatum.probes } - override fun addProbePositions(classId: Long, probePositions: Map>) { - classProbePositions[classId] = probePositions + override fun addClassMethodsMetadata(classId: Long, methodsMetadata: ClassMethodsMetadata) { + classMethodsMetadata[classId] = methodsMetadata } override fun pollRecorded(): Sequence { diff --git a/test2code/src/main/kotlin/com/epam/drill/agent/test2code/coverage/CoverageSender.kt b/test2code/src/main/kotlin/com/epam/drill/agent/test2code/coverage/CoverageSender.kt index 7c251867..8fec6feb 100644 --- a/test2code/src/main/kotlin/com/epam/drill/agent/test2code/coverage/CoverageSender.kt +++ b/test2code/src/main/kotlin/com/epam/drill/agent/test2code/coverage/CoverageSender.kt @@ -42,7 +42,7 @@ class IntervalCoverageSender( private val sender: AgentMessageSender = StubSender(), private val collectReleasedProbes: () -> Sequence = { emptySequence() }, private val collectUnreleasedProbes: () -> Sequence = { emptySequence() }, - private val classProbePositions: ConcurrentHashMap>> + private val classMethodsMetadata: ConcurrentHashMap ) : CoverageSender { private val scheduledThreadPool = Executors.newSingleThreadScheduledExecutor() private val destination = AgentMessageDestination("POST", "coverage") @@ -77,29 +77,25 @@ class IntervalCoverageSender( */ private fun sendProbes(dataToSend: Sequence) { dataToSend - .mapNotNull { - classProbePositions[it.id]?.let { positionsByMethod -> - it to positionsByMethod - } ?: run { - logger.warn("No probe positions for class id=${it.id}") - null - } - } - .flatMap { (datum, positionsByMethod) -> - positionsByMethod.mapNotNull { (signature, positions) -> - val methodProbes = - datum.probes.values - .copyOfRange(positions.first, positions.first + positions.second) - .toBitSet() + .flatMap { + classMethodsMetadata[it.id] + ?.mapNotNull { (signature, metadata) -> + val methodProbes = it.probes.values.copyOfRange( + metadata.probesStartPos, + metadata.probesStartPos + metadata.probesCount + ).toBitSet() - if (methodProbes.isEmpty) null - else MethodCoverage( - signature = signature, - testId = datum.testId, - testSessionId = datum.sessionId, - probes = methodProbes - ) - } + if (methodProbes.isEmpty) null + else MethodCoverage( + signature = signature, + bodyChecksum = metadata.bodyChecksum, + testId = it.testId, + testSessionId = it.sessionId, + probes = methodProbes + ) + } + ?.asSequence() + ?: emptySequence() } .chunked(pageSize) .forEach { sender.send(destination, CoveragePayload( diff --git a/test2code/src/main/kotlin/com/epam/drill/agent/test2code/coverage/Instrumentation.kt b/test2code/src/main/kotlin/com/epam/drill/agent/test2code/coverage/Instrumentation.kt index ea5c8114..59bec08e 100644 --- a/test2code/src/main/kotlin/com/epam/drill/agent/test2code/coverage/Instrumentation.kt +++ b/test2code/src/main/kotlin/com/epam/drill/agent/test2code/coverage/Instrumentation.kt @@ -22,7 +22,7 @@ import com.epam.drill.agent.jacoco.DrillClassProbesAdapter import com.epam.drill.agent.jacoco.DrillDuplicateFrameEliminator import com.epam.drill.agent.jacoco.DrillMethodInstrumenter import com.epam.drill.agent.test2code.classparsing.ClassProbeCounter -import com.epam.drill.agent.test2code.classparsing.ProbeCounter +import com.epam.drill.agent.test2code.classparsing.calculateMethodsChecksums import org.jacoco.core.internal.data.CRC64 import org.jacoco.core.internal.flow.* import org.jacoco.core.internal.instr.* @@ -47,9 +47,17 @@ class DrillInstrumenter( val counter = ClassProbeCounter(className) reader.accept(DrillClassProbesAdapter(counter, false), 0) - probesProxy.addProbePositions(classId, counter.methods.associate { m -> - "${m.classname}:${m.name}:${m.params}:${m.returnType}" to Pair(m.probesStartPos, m.probesCount) - }) + val bodyChecksums = calculateMethodsChecksums(initialBytes, className) + val classMethodsMetadata: ClassMethodsMetadata = counter.methods.associate { m -> + val signature = "${m.classname}:${m.name}:${m.params}:${m.returnType}" + signature to ClassMethodMetadata( + probesStartPos = m.probesStartPos, + probesCount = m.probesCount, + bodyChecksum = bodyChecksums[signature] ?: "" // interface methods don't have a body + ) + } + + probesProxy.addClassMethodsMetadata(classId, classMethodsMetadata) val genId = classCounter.incrementAndGet() val probeCount = counter.count diff --git a/test2code/src/main/kotlin/com/epam/drill/agent/test2code/coverage/ProbesProvider.kt b/test2code/src/main/kotlin/com/epam/drill/agent/test2code/coverage/ProbesProvider.kt index 7d056a94..ebefebba 100644 --- a/test2code/src/main/kotlin/com/epam/drill/agent/test2code/coverage/ProbesProvider.kt +++ b/test2code/src/main/kotlin/com/epam/drill/agent/test2code/coverage/ProbesProvider.kt @@ -24,9 +24,16 @@ import com.epam.drill.agent.jacoco.AgentProbes interface IProbesProxy { fun invoke(id: ClassId, num: Int, name: String, probeCount: Int): AgentProbes - fun addProbePositions(classId: Long, probePositions: Map>) + fun addClassMethodsMetadata(classId: Long, methodsMetadata: ClassMethodsMetadata) } +typealias ClassMethodsMetadata = Map +data class ClassMethodMetadata( + val probesStartPos: Int, + val probesCount: Int, + val bodyChecksum: String +) + const val SESSION_CONTEXT_NONE = "SESSION_CONTEXT_NONE" const val TEST_CONTEXT_NONE = "TEST_CONTEXT_NONE" const val SESSION_CONTEXT_AMBIENT = "GLOBAL" diff --git a/test2code/src/test/java/com/epam/drill/agent/fixture/ast/ConstructorTestBuild1.java b/test2code/src/test/java/com/epam/drill/agent/fixture/ast/ConstructorTestBuild1.java deleted file mode 100644 index a96b6211..00000000 --- a/test2code/src/test/java/com/epam/drill/agent/fixture/ast/ConstructorTestBuild1.java +++ /dev/null @@ -1,41 +0,0 @@ -/** - * Copyright 2020 - 2022 EPAM Systems - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package com.epam.drill.agent.fixture.ast; - - -import java.util.UUID; - -public class ConstructorTestBuild1 { - private String id; - private String email; - private String username; - private String password; - private String bio; - private String image; - - public ConstructorTestBuild1(String email, String username, String password, String bio, String image) { - this.id = UUID.randomUUID().toString(); - this.email = email; - this.username = username; - this.password = password; - this.bio = bio; - this.image = image; - } - - public ConstructorTestBuild1() { - - } -} diff --git a/test2code/src/test/java/com/epam/drill/agent/fixture/ast/ConstructorTestBuild2.java b/test2code/src/test/java/com/epam/drill/agent/fixture/ast/ConstructorTestBuild2.java deleted file mode 100644 index 6a7cc788..00000000 --- a/test2code/src/test/java/com/epam/drill/agent/fixture/ast/ConstructorTestBuild2.java +++ /dev/null @@ -1,43 +0,0 @@ -/** - * Copyright 2020 - 2022 EPAM Systems - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package com.epam.drill.agent.fixture.ast; - - -import java.util.UUID; - -public class ConstructorTestBuild2 { - private String id; - private String email; - private String username; - private String password; - private String bio; - private String image; - private String phone; - - public ConstructorTestBuild2(String email, String username, String password, String bio, String image) { - this.id = UUID.randomUUID().toString(); - this.email = email; - this.username = username; - this.password = password; - this.bio = bio; - this.image = image; - this.phone = ""; - } - - public ConstructorTestBuild2() { - - } -} diff --git a/test2code/src/test/kotlin/com/epam/drill/agent/test2code/classparsing/ChecksumTest.kt b/test2code/src/test/kotlin/com/epam/drill/agent/test2code/classparsing/ChecksumTest.kt index d37c0bfd..c37d25c1 100644 --- a/test2code/src/test/kotlin/com/epam/drill/agent/test2code/classparsing/ChecksumTest.kt +++ b/test2code/src/test/kotlin/com/epam/drill/agent/test2code/classparsing/ChecksumTest.kt @@ -20,132 +20,134 @@ import kotlin.test.assertNotEquals import kotlin.test.Test import com.epam.drill.agent.fixture.ast.* import com.epam.drill.agent.fixture.ast.OverrideTest -// TODO -// -make test cases independent (split fixture class into multiple classes) -// -rewrite tests cases testing change ("Build1" and "Build2") + +// TODO these are bad tests: lots of repetition, poorly prepared test data (Build1.java, Build2.java) +// rewrite to actually test cases handled in codeToString class ChecksumTest { private val checksumsBuild1 = calculateMethodsChecksums(Build1::class.readBytes(), Build1::class.getFullName()) private val checksumsBuild2 = calculateMethodsChecksums(Build2::class.readBytes(), Build2::class.getFullName()) @Test - fun `lambda with different context should have other checksum`() { - val methodName = "lambda\$differentContextChangeAtLambda\$1/java.lang.String/java.lang.String" - // Lambdas should have different value, because they contain different context - assertChecksum(methodName, checksumsBuild1, checksumsBuild2) - } - - @Test - fun `constant lambda hash calculation`() { - val methodName = "lambda\$constantLambdaHashCalculation\$6/java.lang.String/java.lang.String" + fun `lambda with no changes to body should have matching checksums`() { + val methodSignatureBuild1 = "com/epam/drill/agent/fixture/ast/Build1:lambda\$constantLambdaHashCalculation\$6:java.lang.String:java.lang.String" + val methodSignatureBuild2 = "com/epam/drill/agent/fixture/ast/Build2:lambda\$constantLambdaHashCalculation\$6:java.lang.String:java.lang.String" assertEquals( - checksumsBuild1[methodName], - checksumsBuild2[methodName] + checksumsBuild1[methodSignatureBuild1], + checksumsBuild2[methodSignatureBuild2] ) } @Test - fun `method with lambda should have the same checksum`() { - val methodName = "theSameMethodBody" - //Methods, which contain lambda with different context, should not have difference in checksum + fun `difference in nested lambdas should not change method body checksum`() { + val methodSignatureBuild1 = "com/epam/drill/agent/fixture/ast/Build1:theSameMethodBody:java.util.List:void" + val methodSignatureBuild2 = "com/epam/drill/agent/fixture/ast/Build2:theSameMethodBody:java.util.List:void" assertEquals( - checksumsBuild1[methodName], - checksumsBuild2[methodName] + checksumsBuild1[methodSignatureBuild1], + checksumsBuild2[methodSignatureBuild2] ) } @Test - fun `test on different context at inner lambda`() { - val methodName = "lambda\$null\$2/java.lang.String/java.lang.String" - assertChecksum(methodName, checksumsBuild1, checksumsBuild2) + fun `changing lambda body should change it's checksum`() { + val methodSignatureBuild1 = "com/epam/drill/agent/fixture/ast/Build1:lambda\$null\$2:java.lang.String:java.lang.String" + val methodSignatureBuild2 = "com/epam/drill/agent/fixture/ast/Build2:lambda\$null\$2:java.lang.String:java.lang.String" + assertNotEquals( + checksumsBuild1[methodSignatureBuild1], + checksumsBuild2[methodSignatureBuild2] + ) } + // TODO this test actually performs check on a different _object_ and not just the method called by reference @Test fun `should have different checksum for reference call`() { - val methodName = "referenceMethodCall/java.util.List/void" - assertChecksum(methodName, checksumsBuild1, checksumsBuild2) + val methodSignatureBuild1 = "com/epam/drill/agent/fixture/ast/Build1:referenceMethodCall:java.util.List:void" + val methodSignatureBuild2 = "com/epam/drill/agent/fixture/ast/Build2:referenceMethodCall:java.util.List:void" + assertNotEquals( + checksumsBuild1[methodSignatureBuild1], + checksumsBuild2[methodSignatureBuild2] + ) } + // TODO this test changes so much in the method, I'm not sure what exactly are we testing here + // either rewrite or remove it @Test fun `should have different checksum for array`() { - val methodName = "multiANewArrayInsnNode//void" - assertChecksum(methodName, checksumsBuild1, checksumsBuild2) + val methodSignatureBuild1 = "com/epam/drill/agent/fixture/ast/Build1:multiANewArrayInsnNode::void" + val methodSignatureBuild2 = "com/epam/drill/agent/fixture/ast/Build2:multiANewArrayInsnNode::void" + assertNotEquals( + checksumsBuild1[methodSignatureBuild1], + checksumsBuild2[methodSignatureBuild2] + ) } + // TODO this test and the next one test arguably the same behavior @Test - fun `should have different checksum for switch table`() { - val methodName = "tableSwitchMethodTest/int/void" - assertChecksum(methodName, checksumsBuild1, checksumsBuild2) + fun `changing rule in look up switch should change checksum`() { + val methodSignatureBuild1 = "com/epam/drill/agent/fixture/ast/Build1:tableSwitchMethodTest:int:void" + val methodSignatureBuild2 = "com/epam/drill/agent/fixture/ast/Build2:tableSwitchMethodTest:int:void" + assertNotEquals( + checksumsBuild1[methodSignatureBuild1], + checksumsBuild2[methodSignatureBuild2] + ) } @Test - fun `should have different checksum for look up switch`() { - val methodName = "lookupSwitchMethodTest/java.lang.Integer/void" - assertChecksum(methodName, checksumsBuild1, checksumsBuild2) + fun `changing rule in look up switch should change checksum 2`() { + val methodSignatureBuild1 = "com/epam/drill/agent/fixture/ast/Build1:lookupSwitchMethodTest:java.lang.Integer:void" + val methodSignatureBuild2 = "com/epam/drill/agent/fixture/ast/Build2:lookupSwitchMethodTest:java.lang.Integer:void" + assertNotEquals( + checksumsBuild1[methodSignatureBuild1], + checksumsBuild2[methodSignatureBuild2] + ) } @Test - fun `method with different instance type`() { - val methodName = "differentInstanceType//void" - assertChecksum(methodName, checksumsBuild1, checksumsBuild2) + fun `changing local variable type should change checksum`() { + val methodSignatureBuild1 = "com/epam/drill/agent/fixture/ast/Build1:differentInstanceType::void" + val methodSignatureBuild2 = "com/epam/drill/agent/fixture/ast/Build2:differentInstanceType::void" + assertNotEquals( + checksumsBuild1[methodSignatureBuild1], + checksumsBuild2[methodSignatureBuild2] + ) } + // TODO no idea why these checksums should be different @Test fun `method call other method`() { - val methodName = "callOtherMethod//void" - assertChecksum(methodName, checksumsBuild1, checksumsBuild2) + val methodSignatureBuild1 = "com/epam/drill/agent/fixture/ast/Build1:callOtherMethod::void" + val methodSignatureBuild2 = "com/epam/drill/agent/fixture/ast/Build2:callOtherMethod::void" + assertNotEquals( + checksumsBuild1[methodSignatureBuild1], + checksumsBuild2[methodSignatureBuild2] + ) } -// Do we need to cover that case? -// @Test -// fun `method with different params`() { -// val methodName = "methodWithDifferentParams" -// assertChecksum(methodName) -// } + // TODO not sure it _should_ work like that, but that's what our codeToString implementation does @Test - fun `method with incremented value`() { - val methodName = "methodWithIteratedValue/java.lang.Integer/void" - assertChecksum(methodName, checksumsBuild1, checksumsBuild2) + fun `changing operator ++ to += should change checksum`() { + val methodSignatureBuild1 = "com/epam/drill/agent/fixture/ast/Build1:methodWithIteratedValue:java.lang.Integer:void" + val methodSignatureBuild2 = "com/epam/drill/agent/fixture/ast/Build2:methodWithIteratedValue:java.lang.Integer:void" + assertNotEquals( + checksumsBuild1[methodSignatureBuild1], + checksumsBuild2[methodSignatureBuild2] + ) } -// Do we need to cover that case? -// @Test -// fun `change name of local var`() { -// val methodName = "changeLocalVarName" -// assertChecksum(methodName) -// } - } -class ConstructorTest { - - private val checksumsBuild1 = - calculateMethodsChecksums(ConstructorTestBuild1::class.readBytes(), ConstructorTestBuild1::class.getFullName()) - private val checksumsBuild2 = - calculateMethodsChecksums(ConstructorTestBuild2::class.readBytes(), ConstructorTestBuild2::class.getFullName()) - - @Test - fun `class with more that one constructor should have two different checksum`() { - val constructorName = - "/java.lang.String,java.lang.String,java.lang.String,java.lang.String,java.lang.String/void" - val defaultConstructorName = "//void" - assertChecksum(constructorName, checksumsBuild1, checksumsBuild2) - assertEquals( - checksumsBuild1[defaultConstructorName], - checksumsBuild2[defaultConstructorName] - ) - } -} class OverrideTest { private val methodsBuild = calculateMethodsChecksums(OverrideTest::class.readBytes(), OverrideTest::class.getFullName()) + // TODO I really doubt that - OverrideTest changes method _body_ + // it has very little to do with methods being virtual/overwritten @Test - fun `class with override methods should have different checksum`() { - val methodNameReal = "call//java.lang.String" - val methodNameVirtual = "call//java.lang.Object" + fun `virtual and overwritten method should have different body checksums`() { + val methodNameReal = "com/epam/drill/agent/fixture/ast/OverrideTest:call::java.lang.String" + val methodNameVirtual = "com/epam/drill/agent/fixture/ast/OverrideTest:call::java.lang.Object" assertEquals(3, methodsBuild.size) //Compare checksum of virtual method and method with override annotation assertNotEquals( @@ -155,13 +157,3 @@ class OverrideTest { } } -private fun assertChecksum( - methodName: String, - build1: Map, - build2: Map -) { - assertNotEquals( - build1[methodName], - build2[methodName] - ) -}