Added save-overrides.toml (#438)

* introduced `save-overrides.toml` * removed `--override-exec-cmd` and `--override-exec-flags` from cli * moved `batchSize` and `batchSeparator` from cli to save.toml
saveourtool · Aug 29, 2022 · a7809a8 · a7809a8
1 parent c1c6eaa
commit a7809a8
Show file tree

Hide file tree

Showing 33 changed files with 911 additions and 521 deletions.
diff --git a/OptionsTable.md b/OptionsTable.md
@@ -12,8 +12,4 @@ 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 |
-| - | 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) | ,  |
+| - | report-dir | Path to directory, where to store output (when `resultOutput` is set to `FILE`) | save-reports |
diff --git a/buildSrc/src/main/resources/config-options.json b/buildSrc/src/main/resources/config-options.json
@@ -78,37 +78,5 @@
       "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" : ", "
     }
 }
diff --git a/save-cli/src/commonNonJsMain/kotlin/com/saveourtool/save/cli/config/SavePropertiesExt.kt b/save-cli/src/commonNonJsMain/kotlin/com/saveourtool/save/cli/config/SavePropertiesExt.kt
@@ -7,7 +7,6 @@ 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
@@ -52,24 +51,6 @@ 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
 }
 

diff --git a/save-common-test/src/commonNonJsMain/kotlin/com/saveourtool/save/plugin/MockPlugin.kt b/save-common-test/src/commonNonJsMain/kotlin/com/saveourtool/save/plugin/MockPlugin.kt
@@ -1,6 +1,5 @@
 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
@@ -15,13 +14,17 @@ internal expect val fs: FileSystem
  * No-op implementation of [Plugin] that can be used to test reporters, which expect only a class name of the plugin.
  */
 class MockPlugin(baseDir: Path, testFiles: List<String> = emptyList()) : Plugin(
-    TestConfig((baseDir / "save.toml").also { fs.createFile(it) }, null, fs = fs),
+    TestConfig(
+        (baseDir / "save.toml").also { fs.createFile(it) },
+        null,
+        overridesPluginConfigs = emptyList(),
+        fs = fs),
     testFiles,
     fs,
     useInternalRedirections = true,
     redirectTo = null
 ) {
-    override fun handleFiles(evaluatedToolConfig: EvaluatedToolConfig, files: Sequence<TestFiles>): Sequence<TestResult> = emptySequence()
+    override fun handleFiles(files: Sequence<TestFiles>): Sequence<TestResult> = emptySequence()
 
     override fun rawDiscoverTestFiles(resourceDirectories: Sequence<Path>): Sequence<TestFiles> = emptySequence()
 

diff --git a/save-common/src/commonMain/kotlin/com/saveourtool/save/core/config/EvaluatedToolConfig.kt b/save-common/src/commonMain/kotlin/com/saveourtool/save/core/config/EvaluatedToolConfig.kt
diff --git a/save-common/src/commonMain/kotlin/com/saveourtool/save/core/config/TestConfig.kt b/save-common/src/commonMain/kotlin/com/saveourtool/save/core/config/TestConfig.kt
@@ -10,24 +10,30 @@ import com.saveourtool.save.core.logging.logTrace
 import com.saveourtool.save.core.plugin.GeneralConfig
 import com.saveourtool.save.core.plugin.Plugin
 import com.saveourtool.save.core.plugin.PluginConfig
+import com.saveourtool.save.core.utils.mergeWith
+import com.saveourtool.save.core.utils.overrideBy
+import com.saveourtool.save.core.utils.singleIsInstanceOrNull
 
 import okio.FileSystem
 import okio.Path
 import okio.Path.Companion.toPath
+
 import kotlin.js.JsName
 
 /**
  * Configuration for a test suite, that is read from test suite configuration file (toml config)
  * @property location [Path] denoting the location of this file
  * @property parentConfig parent config in the hierarchy of configs, `null` if this config is root.
  * @property pluginConfigs list of configurations for plugins that are active in this config
+ * @property overridesPluginConfigs list of configurations for plugins that overrides [pluginConfigs]
  * @property fs filesystem which can access test configs
  */
 @Suppress("TYPE_ALIAS", "TooManyFunctions")
 data class TestConfig(
     val location: Path,
     val parentConfig: TestConfig?,
     val pluginConfigs: MutableList<PluginConfig> = mutableListOf(),
+    val overridesPluginConfigs: List<PluginConfig>,
     val fs: FileSystem,
 ) {
     /**
@@ -78,7 +84,7 @@ data class TestConfig(
      *
      * @return [GeneralConfig] or `null` if not found
      */
-    fun getGeneralConfig() = pluginConfigs.filterIsInstance<GeneralConfig>().singleOrNull()
+    fun getGeneralConfig(): GeneralConfig? = pluginConfigs.singleIsInstanceOrNull()
 
     /**
      * @param withSelf if true, include this config as the first element of the sequence or start with parent config otherwise
@@ -122,15 +128,17 @@ data class TestConfig(
      * @return an update this [TestConfig]
      */
     fun processInPlace(createPluginConfigList: (TestConfig) -> List<PluginConfig>): TestConfig {
-        // need to process parent first
-        this.parentConfig?.processInPlace(createPluginConfigList)
         // discover plugins from the test configuration
-        createPluginConfigList(this).forEach {
-            logTrace("Discovered new pluginConfig: $it")
-            this.pluginConfigs.merge(it)
+        createPluginConfigList(this).forEach { pluginConfig ->
+            logTrace("Discovered new pluginConfig: $pluginConfig")
+            require(this.pluginConfigs.none { it.type == pluginConfig.type }) {
+                "Found duplicate for $pluginConfig"
+            }
+            this.pluginConfigs.add(pluginConfig)
         }
         // merge configurations with parents
-        this.mergeConfigWithParent()
+        mergeConfigWithParent()
+        overrideConfig()
         return this
     }
 
@@ -161,27 +169,28 @@ data class TestConfig(
      *
      * @return all plugin configs without general config
      */
-    fun pluginConfigsWithoutGeneralConfig() = pluginConfigs.filterNot { it is GeneralConfig }
+    private fun pluginConfigsWithoutGeneralConfig() = pluginConfigs.filterNot { it is GeneralConfig }
 
     /**
      * Merge parent list of plugins with the current list
      *
      * @return merged test config
      */
-    fun mergeConfigWithParent(): TestConfig {
-        logDebug("Merging configs  (with parental configs from higher directory level) for ${this.location}")
+    private fun mergeConfigWithParent(): TestConfig {
+        logDebug("Merging configs (with parental configs from higher directory level) for ${this.location}")
 
-        if (parentConfig != null) {
+        parentConfig?.let {
             logTrace("Using parental config ${parentConfig.location} to merge it with child config: ${this.location}")
-            // return from the function if we stay at the root element of the plugin tree
-            val parentalPlugins = parentConfig.pluginConfigs
-            parentalPlugins.forEach { parentalPluginConfig ->
-                this.pluginConfigs.merge(parentalPluginConfig)
-            }
+            this.pluginConfigs.mergeWith(parentConfig.pluginConfigs)
         }
         return this
     }
 
+    private fun overrideConfig() {
+        logDebug("Overriding configs for $location")
+        pluginConfigs.overrideBy(overridesPluginConfigs)
+    }
+
     /**
      * Method, which validates all plugin configs and set default values, if possible
      */
@@ -192,23 +201,6 @@ data class TestConfig(
         logDebug("Validated plugin configuration for [$location] " +
                 "(${pluginConfigs.map { it.type }.filterNot { it == TestConfigSections.GENERAL }})")
     }
-
-    private fun MutableList<PluginConfig>.merge(parentalPluginConfig: PluginConfig) {
-        val childConfigs = this.filter { it.type == parentalPluginConfig.type }
-        if (childConfigs.isEmpty()) {
-            // if we haven't found a plugin from parent in a current list of plugins - we will simply copy it
-            this.add(parentalPluginConfig)
-        } else {
-            require(childConfigs.size == 1) {
-                "Duplicate config with type ${parentalPluginConfig.type} in $this"
-            }
-            val childConfig = childConfigs.single()
-            // else, we will merge plugin with a corresponding plugin from a parent config
-            // we expect that there is only one plugin of such type, otherwise we will throw an exception
-            logTrace("Merging process of ${parentalPluginConfig.type} from $parentalPluginConfig into $childConfig")
-            this[this.indexOf(childConfig)] = childConfig.mergeWith(parentalPluginConfig)
-        }
-    }
 }
 
 /**
@@ -235,3 +227,8 @@ fun Path.isSaveTomlConfig() = name == "save.toml"
  * @return a file (save.toml) in current directory
  */
 fun Path.resolveSaveTomlConfig() = this / "save.toml"
+
+/**
+ * @return a file (save-overrides.toml) in current directory
+ */
+fun Path.resolveSaveOverridesTomlConfig() = this / "save-overrides.toml"
diff --git a/save-common/src/commonMain/kotlin/com/saveourtool/save/core/plugin/Plugin.kt b/save-common/src/commonMain/kotlin/com/saveourtool/save/core/plugin/Plugin.kt
@@ -1,6 +1,5 @@
 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
@@ -12,6 +11,7 @@ import com.saveourtool.save.core.result.Ignored
 import com.saveourtool.save.core.result.TestResult
 import com.saveourtool.save.core.utils.PathSerializer
 import com.saveourtool.save.core.utils.ProcessBuilder
+import com.saveourtool.save.core.utils.singleIsInstanceOrNull
 
 import okio.FileSystem
 import okio.Path
@@ -44,17 +44,15 @@ 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(evaluatedToolConfig: EvaluatedToolConfig): Sequence<TestResult> {
+    fun execute(): Sequence<TestResult> {
         clean()
         val testFilesList = discoverTestFiles(testConfig.directory).toList()
 
         val excludedTests = testConfig
             .pluginConfigs
-            .filterIsInstance<GeneralConfig>()
-            .singleOrNull()
+            .singleIsInstanceOrNull<GeneralConfig>()
             ?.excludedTests
 
         if (!excludedTests.isNullOrEmpty()) {
@@ -71,7 +69,7 @@ abstract class Plugin(
             val excludedTestResults = excludedTestFiles.map {
                 TestResult(it, Ignored("Excluded by configuration"))
             }
-            handleFiles(evaluatedToolConfig, actualTestFiles.asSequence()) + excludedTestResults
+            handleFiles(actualTestFiles.asSequence()) + excludedTestResults
         } else {
             emptySequence()
         }
@@ -80,11 +78,10 @@ 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(evaluatedToolConfig: EvaluatedToolConfig, files: Sequence<TestFiles>): Sequence<TestResult>
+    abstract fun handleFiles(files: Sequence<TestFiles>): Sequence<TestResult>
 
     /**
      * Discover groups of resource files which will be used to run tests, applying additional filtering