Skip to content

Commit

Permalink
execCmd and batchSize on cli args (#433)
Browse files Browse the repository at this point in the history
* added cli args
* added EvaluatedToolConfig
  • Loading branch information
nulls committed Aug 22, 2022
1 parent f064588 commit ff844eb
Show file tree
Hide file tree
Showing 24 changed files with 186 additions and 90 deletions.
6 changes: 5 additions & 1 deletion OptionsTable.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,8 @@ Most (except for `-h` and `-prop`) of the options below can be passed to a SAVE
| i | include-suites | Test suites, only which ones will be checked | - |
| l | language | Language that you are developing analyzer for | UNDEFINED |
| out | result-output | Data output stream | STDOUT |
| - | report-dir | Path to directory, where to store output (when `resultOutput` is set to `FILE`) | save-reports |
| - | report-dir | Path to directory, where to store output (when `resultOutput` is set to `FILE`) | save-reports |
| - | override-exec-cmd | A temporary workaround for save-cloud to override `execCmd` in `save.toml` | - |
| - | override-exec-flags | A temporary workaround for save-cloud to override `execFlags` in `save.toml` | - |
| - | batch-size | Number of files execCmd will process at a time | 1 |
| - | batch-separator | A separator to join test files to string if the tested tool supports processing of file batches (`batch-size` > 1) | , |
32 changes: 32 additions & 0 deletions buildSrc/src/main/resources/config-options.json
Original file line number Diff line number Diff line change
Expand Up @@ -78,5 +78,37 @@
"shortName" : "",
"description" : "Path to directory, where to store output (when `resultOutput` is set to `FILE`)",
"default" : "save-reports"
},
"overrideExecCmd" : {
"argType": "ArgType.String",
"kotlinType": "kotlin.String",
"fullName": "override-exec-cmd",
"shortName" : "",
"description" : "A temporary workaround for save-cloud to override `execCmd` in `save.toml`",
"default" : null
},
"overrideExecFlags" : {
"argType": "ArgType.String",
"kotlinType": "kotlin.String",
"fullName": "override-exec-flags",
"shortName" : "",
"description" : "A temporary workaround for save-cloud to override `execFlags` in `save.toml`",
"default" : null
},
"batchSize" : {
"argType" : "ArgType.Int",
"kotlinType": "kotlin.Int",
"fullName" : "batch-size",
"shortName" : "",
"description" : "Number of files execCmd will process at a time",
"default" : "1"
},
"batchSeparator" : {
"argType": "ArgType.String",
"kotlinType": "kotlin.String",
"fullName": "batch-separator",
"shortName" : "",
"description" : "A separator to join test files to string if the tested tool supports processing of file batches (`batch-size` > 1)",
"default" : ", "
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package com.saveourtool.save.cli.config
import com.saveourtool.save.cli.ExitCodes
import com.saveourtool.save.cli.fs
import com.saveourtool.save.cli.logging.logErrorAndExit
import com.saveourtool.save.cli.logging.logWarn
import com.saveourtool.save.core.config.SaveProperties
import com.saveourtool.save.core.config.resolveSaveTomlConfig
import com.saveourtool.save.core.logging.logDebug
Expand Down Expand Up @@ -51,6 +52,23 @@ private fun SaveProperties.validate(): SaveProperties {
" Please provide a valid path to the test config via command-line or using the file with properties."
)
}
if (batchSize < 1) {
return logErrorAndExit(
ExitCodes.INVALID_CONFIGURATION, "Property `batch-size` should be more or equal to 1."
)
}
overrideExecCmd?.also {
logWarn {
"Property `override-exec-cmd` is a temporary workaround for `save-cloud`, " +
"please be aware this property can be removed in future versions"
}
}
overrideExecFlags?.also {
logWarn {
"Property `override-exec-flags` is a temporary workaround for `save-cloud`, " +
"please be aware this property can be removed in future versions"
}
}

return this
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,10 @@ import com.saveourtool.save.cli.ExitCodes
*/
@Deprecated("never use this method in save-core as it can lead to a break of save-cloud application")
expect fun logErrorAndExit(exitCode: ExitCodes, message: String): Nothing

/**
* Log result of [messageSupplier] with level WARN
*
* @param messageSupplier supplier for message to log
*/
expect fun logWarn(messageSupplier: () -> String): Unit
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,14 @@ package com.saveourtool.save.cli.logging

import com.saveourtool.save.cli.ExitCodes
import com.saveourtool.save.core.logging.logError
import com.saveourtool.save.core.logging.logWarn
import kotlin.system.exitProcess

actual fun logErrorAndExit(exitCode: ExitCodes, message: String): Nothing {
logError(message)
exitProcess(exitCode.code)
}

actual fun logWarn(messageSupplier: () -> String) {
logWarn(messageSupplier())
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,14 @@ package com.saveourtool.save.cli.logging

import com.saveourtool.save.cli.ExitCodes
import com.saveourtool.save.core.logging.logError
import com.saveourtool.save.core.logging.logWarn
import kotlin.system.exitProcess

actual fun logErrorAndExit(exitCode: ExitCodes, message: String): Nothing {
logError(message)
exitProcess(exitCode.code)
}

actual fun logWarn(messageSupplier: () -> String) {
logWarn(messageSupplier())
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.saveourtool.save.plugin

import com.saveourtool.save.core.config.EvaluatedToolConfig
import com.saveourtool.save.core.config.TestConfig
import com.saveourtool.save.core.files.createFile
import com.saveourtool.save.core.plugin.Plugin
Expand All @@ -20,7 +21,7 @@ class MockPlugin(baseDir: Path, testFiles: List<String> = emptyList()) : Plugin(
useInternalRedirections = true,
redirectTo = null
) {
override fun handleFiles(files: Sequence<TestFiles>): Sequence<TestResult> = emptySequence()
override fun handleFiles(evaluatedToolConfig: EvaluatedToolConfig, files: Sequence<TestFiles>): Sequence<TestResult> = emptySequence()

override fun rawDiscoverTestFiles(resourceDirectories: Sequence<Path>): Sequence<TestFiles> = emptySequence()

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package com.saveourtool.save.core.config

/**
* Configuration for an evaluated tool, that is read from test suite configuration file (toml config) or cli
*
* @property execCmd
* @property execFlags
* @property batchSize it controls how many files execCmd will process at a time
* @property batchSeparator A separator to join test files to string if the tested tool supports processing of file batches (`batch-size` > 1)
*/
data class EvaluatedToolConfig(
val execCmd: String?,
val execFlags: String?,
val batchSize: Int,
val batchSeparator: String,
)
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.saveourtool.save.core.plugin

import com.saveourtool.save.core.config.EvaluatedToolConfig
import com.saveourtool.save.core.config.TestConfig
import com.saveourtool.save.core.config.isSaveTomlConfig
import com.saveourtool.save.core.files.createRelativePathToTheRoot
Expand All @@ -20,6 +21,7 @@ import kotlinx.serialization.Serializable

/**
* Plugin that can be injected into SAVE during execution. Plugins accept contents of configuration file and then perform some work.
*
* @property testConfig
* @property testFiles a list of files (test resources or save.toml configs)
* @property fs describes the current file system
Expand All @@ -42,9 +44,10 @@ abstract class Plugin(
/**
* Perform plugin's work.
*
* @param evaluatedToolConfig a configuration for evaluated tool
* @return a sequence of [TestResult]s for each group of test resources
*/
fun execute(): Sequence<TestResult> {
fun execute(evaluatedToolConfig: EvaluatedToolConfig): Sequence<TestResult> {
clean()
val testFilesList = discoverTestFiles(testConfig.directory).toList()

Expand All @@ -68,7 +71,7 @@ abstract class Plugin(
val excludedTestResults = excludedTestFiles.map {
TestResult(it, Ignored("Excluded by configuration"))
}
handleFiles(actualTestFiles.asSequence()) + excludedTestResults
handleFiles(evaluatedToolConfig, actualTestFiles.asSequence()) + excludedTestResults
} else {
emptySequence()
}
Expand All @@ -77,10 +80,11 @@ abstract class Plugin(
/**
* Perform plugin's work on a set of files.
*
* @param evaluatedToolConfig a configuration for evaluated tool
* @param files a sequence of file groups, corresponding to tests.
* @return a sequence of [TestResult]s for each group of test resources
*/
abstract fun handleFiles(files: Sequence<TestFiles>): Sequence<TestResult>
abstract fun handleFiles(evaluatedToolConfig: EvaluatedToolConfig, files: Sequence<TestFiles>): Sequence<TestResult>

/**
* Discover groups of resource files which will be used to run tests, applying additional filtering
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ interface PluginConfig {
val resourceNamePatternStr: String

/**
* @param otherConfig - 'this' will be merged with 'other'
* @param otherConfig 'this' will be merged with 'other'
* @return merged config
*/
fun mergeWith(otherConfig: PluginConfig): PluginConfig
Expand All @@ -60,13 +60,13 @@ interface PluginConfig {
* of nested configs, we can't detect whether the value are passed by user, or taken from default.
* The logic of the default value processing will be provided in stage of validation
*
* @property execCmd a command that will be executed to check resources and emit warnings
* @property execCmd a command that will be executed to check resources
* @property tags special labels that can be used for splitting tests into groups
* @property description free text with a description
* @property suiteName name of test suite that can be visible from save-cloud
* @property language to tests
* @property excludedTests excluded tests from the run
* @property expectedWarningsPattern - pattern with warnings that are expected from the test file
* @property expectedWarningsPattern pattern with warnings that are expected from the test file
* @property runConfigPattern everything from the capture group will be split by comma and then by `=`
* @property timeOutMillis command execution time for one test
* @property expectedWarningsMiddlePattern
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,18 @@

package com.saveourtool.save.core

import com.saveourtool.save.core.files.fs
import com.saveourtool.save.core.utils.ProcessBuilder
import com.saveourtool.save.core.utils.ProcessBuilder.Companion.processCommandWithEcho
import com.saveourtool.save.core.utils.ProcessExecutionException
import com.saveourtool.save.core.utils.isCurrentOsWindows

import okio.FileSystem

import kotlin.test.Test
import kotlin.test.assertEquals

@Suppress("INLINE_CLASS_CAN_BE_USED")
class ProcessBuilderTest {
private val processBuilder = ProcessBuilder(useInternalRedirections = true, FileSystem.SYSTEM)
private val processBuilder = ProcessBuilder(useInternalRedirections = true, fs)

@Test
fun `empty command`() {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.saveourtool.save.core

import com.saveourtool.save.core.config.EvaluatedToolConfig
import com.saveourtool.save.core.config.OutputStreamType
import com.saveourtool.save.core.config.ReportType
import com.saveourtool.save.core.config.SAVE_VERSION
Expand Down Expand Up @@ -72,6 +73,14 @@ class Save(

reporter.beforeAll()

// create config for evaluated tool from cli args
val evaluatedToolConfig = EvaluatedToolConfig(
execCmd = saveProperties.overrideExecCmd,
execFlags = saveProperties.overrideExecFlags,
batchSize = saveProperties.batchSize,
batchSeparator = saveProperties.batchSeparator,
)

// get all toml configs in file system
val testConfigs = ConfigDetector(fs)
.configFromFile(rootTestConfigPath)
Expand All @@ -97,7 +106,7 @@ class Save(
?.forEach {
atLeastOneExecutionProvided = true
// execute created plugins
executePlugin(it, reporter)
executePlugin(evaluatedToolConfig, it, reporter)
}
?.also {
reporter.onSuiteEnd(testConfig.getGeneralConfig()?.suiteName!!)
Expand Down Expand Up @@ -146,12 +155,16 @@ class Save(
(excludeSuites.isEmpty() || !excludeSuites.contains(suiteName))
}

private fun executePlugin(plugin: Plugin, reporter: Reporter) {
private fun executePlugin(
evaluatedToolConfig: EvaluatedToolConfig,
plugin: Plugin,
reporter: Reporter
) {
reporter.onPluginInitialization(plugin)
logDebug("=> Executing plugin: ${plugin::class.simpleName} for [${plugin.testConfig.location}]")
reporter.onPluginExecutionStart(plugin)
try {
plugin.execute()
plugin.execute(evaluatedToolConfig)
.onEach { event ->
// calculate relative paths, because reporters don't need paths higher than root dir
val resourcesRelative = event.resources.withRelativePaths(testRootPath)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,16 +40,16 @@ class MergeConfigsTest {
private val warningsOutputPattern1 = Regex(".*")
private val warningsOutputPattern2 = Regex("\\w+ - (\\d+)/(\\d+) - (.*)$")
private val warnConfig1 = WarnPluginConfig("execCmd1", warningsOutputPattern2,
false, false, 1, ", ", 1, 1, 1, 1, 1, 1, 1, 1, 1, false, null)
false, false, 1, 1, 1, 1, 1, 1, 1, 1, 1, false, null)
private val warnConfig2 = WarnPluginConfig("execCmd2", warningsOutputPattern1,
true, true, 1, ", ", 2, 2, 2, 1, 1, 2, 2, 2, 2, true, null)
true, true, 2, 2, 2, 1, 1, 2, 2, 2, 2, true, null)
private val warnConfig3 = WarnPluginConfig("execCmd3", warningsOutputPattern2,
warningTextHasColumn = false, batchSize = 1, lineCaptureGroup = 3, columnCaptureGroup = 3, messageCaptureGroup = 3,
warningTextHasColumn = false, lineCaptureGroup = 3, columnCaptureGroup = 3, messageCaptureGroup = 3,
fileNameCaptureGroupOut = 3, lineCaptureGroupOut = 3, columnCaptureGroupOut = 3, messageCaptureGroupOut = 3)
private val warnConfig4 = WarnPluginConfig("execCmd4", warningsOutputPattern2,
batchSize = 1, lineCaptureGroup = 4, columnCaptureGroup = 4, messageCaptureGroup = 4,
lineCaptureGroup = 4, columnCaptureGroup = 4, messageCaptureGroup = 4,
fileNameCaptureGroupOut = 4, lineCaptureGroupOut = 4, columnCaptureGroupOut = 4, messageCaptureGroupOut = 4)
private val fixConfig1 = FixPluginConfig("fixCmd1", 1, "Suffix")
private val fixConfig1 = FixPluginConfig("fixCmd1", "Suffix")
private val fixConfig2 = FixPluginConfig("fixCmd2")
private val fixConfig3 = FixPluginConfig("fixCmd3", null)
private val fixConfig4 = FixPluginConfig("fixCmd4")
Expand Down Expand Up @@ -122,8 +122,8 @@ class MergeConfigsTest {
val expectedGeneralConfig =
GeneralConfig("", listOf("Tag11", "Tag12", "Tag21"), "Description2", "suiteName2", "Kotlin", listOf("excludedTests: test3"), runConfigPattern = extraFlagsPattern1)
val expectedWarnConfig = WarnPluginConfig("execCmd3", warningsOutputPattern2,
true, false, 1, ", ", 3, 3, 3, 1, 1, 3, 3, 3, 3, true, null)
val expectedFixConfig = FixPluginConfig("fixCmd2", 1, "Suffix")
true, false, 3, 3, 3, 1, 1, 3, 3, 3, 3, true, null)
val expectedFixConfig = FixPluginConfig("fixCmd2", "Suffix")

val actualGeneralConfig = config2.pluginConfigs.filterIsInstance<GeneralConfig>().first()
val actualWarnConfig = config2.pluginConfigs.filterIsInstance<WarnPluginConfig>().first()
Expand Down Expand Up @@ -151,8 +151,8 @@ class MergeConfigsTest {
val expectedGeneralConfig =
GeneralConfig("", listOf("Tag11", "Tag12", "Tag21", "Tag31", "Tag32"), "Description2", "suiteName4", "Kotlin", listOf("excludedTests: test7"), runConfigPattern = extraFlagsPattern2)
val expectedWarnConfig = WarnPluginConfig("execCmd4", warningsOutputPattern2,
true, false, 1, ", ", 4, 4, 4, 1, 1, 4, 4, 4, 4, true, null)
val expectedFixConfig = FixPluginConfig("fixCmd4", 1, "Suffix")
true, false, 4, 4, 4, 1, 1, 4, 4, 4, 4, true, null)
val expectedFixConfig = FixPluginConfig("fixCmd4", "Suffix")

val actualGeneralConfig = config4.pluginConfigs.filterIsInstance<GeneralConfig>().first()
val actualWarnConfig = config4.pluginConfigs.filterIsInstance<WarnPluginConfig>().first()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,13 @@ import com.saveourtool.save.core.config.LogType
import com.saveourtool.save.core.config.OutputStreamType
import com.saveourtool.save.core.config.ReportType
import com.saveourtool.save.core.config.SaveProperties
import com.saveourtool.save.core.files.fs
import com.saveourtool.save.core.logging.logType
import com.saveourtool.save.core.result.Fail
import com.saveourtool.save.core.result.Ignored
import com.saveourtool.save.core.result.Pass
import com.saveourtool.save.reporter.test.TestReporter

import okio.FileSystem
import okio.Path

import kotlin.test.assertEquals
Expand Down Expand Up @@ -55,7 +55,7 @@ fun runTestsWithDiktat(
logType.set(LogType.ALL)
// In this test we need to merge with emulated empty save.properties file in aim to use default values,
// since initially all fields are null
val save = Save(saveProperties, FileSystem.SYSTEM)
val save = Save(saveProperties, fs)
val testReporter = save.performAnalysis() as TestReporter

assertEquals(numberOfTests, testReporter.results.size)
Expand Down
Loading

0 comments on commit ff844eb

Please sign in to comment.