Skip to content

Commit

Permalink
Don't recalculate counters in Execution if value is NOT_APPLICABLE (#…
Browse files Browse the repository at this point in the history
…1170)

* Store aggregated metrics in Execution excluding `NOT_APPLICABLE` metrics from individual test executions (until we can calculate scores for other types of plugins too)
* Change logic of dispaly on frontend: don't calculate metrics only if all test executions under an execution are `NOT_APPLICABLE`; otherwise use filtered data from Execution

Related to #1115

Co-authored-by: Nariman Abdullin <[email protected]>
  • Loading branch information
petertrr and nulls authored Sep 6, 2022
1 parent 1ebd285 commit 9e1443e
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import com.saveourtool.save.backend.repository.ExecutionRepository
import com.saveourtool.save.backend.repository.TestExecutionRepository
import com.saveourtool.save.backend.repository.TestRepository
import com.saveourtool.save.backend.utils.secondsToLocalDateTime
import com.saveourtool.save.core.result.CountWarnings
import com.saveourtool.save.domain.TestResultLocation
import com.saveourtool.save.domain.TestResultStatus
import com.saveourtool.save.entities.Execution
Expand All @@ -16,7 +17,6 @@ import com.saveourtool.save.test.TestDto
import com.saveourtool.save.utils.ScoreType
import com.saveourtool.save.utils.calculateScore
import com.saveourtool.save.utils.debug
import com.saveourtool.save.utils.error
import com.saveourtool.save.utils.getLogger

import org.apache.commons.io.FilenameUtils
Expand Down Expand Up @@ -167,10 +167,11 @@ class TestExecutionService(private val testExecutionRepository: TestExecutionRep
"LongMethod",
"MAGIC_NUMBER",
"MagicNumber",
"PARAMETER_NAME_IN_OUTER_LAMBDA",
)
@Transactional
fun saveTestResult(testExecutionsDtos: List<TestExecutionDto>): List<TestExecutionDto> {
log.debug("Saving ${testExecutionsDtos.size} test results from agent ${testExecutionsDtos.first().agentContainerId}")
log.debug { "Saving ${testExecutionsDtos.size} test results from agent ${testExecutionsDtos.first().agentContainerId}" }
// we take agent id only from first element, because all test executions have same execution
val agentContainerId = requireNotNull(testExecutionsDtos.first().agentContainerId) {
"Attempt to save test results without assigned agent. testExecutionDtos=$testExecutionsDtos"
Expand Down Expand Up @@ -212,10 +213,12 @@ class TestExecutionService(private val testExecutionRepository: TestExecutionRep
it.expected = testExecDto.expected
it.unexpected = testExecDto.unexpected

counters.unmatchedChecks += testExecDto.unmatched ?: 0L
counters.matchedChecks += testExecDto.matched ?: 0L
counters.expectedChecks += testExecDto.expected ?: 0L
counters.unexpectedChecks += testExecDto.unexpected ?: 0L
with(counters) {
unmatchedChecks += testExecDto.unmatched.orZeroIfNotApplicable()
matchedChecks += testExecDto.matched.orZeroIfNotApplicable()
expectedChecks += testExecDto.expected.orZeroIfNotApplicable()
unexpectedChecks += testExecDto.unexpected.orZeroIfNotApplicable()
}

testExecutionRepository.save(it)
},
Expand All @@ -229,9 +232,10 @@ class TestExecutionService(private val testExecutionRepository: TestExecutionRep
transactionTemplate.execute {
val execution = executionRepository.findById(executionId).get()
execution.apply {
log.debug("Updating counters in execution id=$executionId: running=$runningTests-${counters.total()}, " +
"passed=$passedTests+${counters.passed}, failed=$failedTests+${counters.failed}, skipped=$skippedTests+${counters.skipped}"
)
log.debug {
"Updating counters in execution id=$executionId: running=$runningTests-${counters.total()}, " +
"passed=$passedTests+${counters.passed}, failed=$failedTests+${counters.failed}, skipped=$skippedTests+${counters.skipped}"
}
runningTests -= counters.total()
passedTests += counters.passed
failedTests += counters.failed
Expand Down Expand Up @@ -352,6 +356,8 @@ class TestExecutionService(private val testExecutionRepository: TestExecutionRep
}
}

private fun Long?.orZeroIfNotApplicable() = this?.takeUnless { CountWarnings.isNotApplicable(it.toInt()) } ?: 0

@Suppress(
"KDOC_NO_CONSTRUCTOR_PROPERTY",
"MISSING_KDOC_ON_FUNCTION",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

package com.saveourtool.save.frontend.components.basic

import com.saveourtool.save.core.result.CountWarnings
import com.saveourtool.save.execution.ExecutionDto
import com.saveourtool.save.execution.ExecutionStatus
import com.saveourtool.save.frontend.externals.fontawesome.faRedo
Expand Down Expand Up @@ -101,25 +100,15 @@ internal class ExecutionStatisticsValues(executionDto: ExecutionDto?) {
?: "0"
this.precisionRate = executionDto
?.let {
if (isAllApplicable(it.matchedChecks, it.unexpectedChecks)) {
"${it.getPrecisionRate()}"
} else {
"N/A"
}
"${it.getPrecisionRate()}"
}
?: "0"
this.recallRate = executionDto
?.let {
if (isAllApplicable(it.matchedChecks, it.unmatchedChecks)) {
"${it.getRecallRate()}"
} else {
"N/A"
}
"${it.getRecallRate()}"
}
?: "0"
}

private fun isAllApplicable(vararg values: Long): Boolean = values.all { !CountWarnings.isNotApplicable(it.toInt()) }
}

/**
Expand Down

0 comments on commit 9e1443e

Please sign in to comment.