From 239c01976ac585c15111d42487ee1e82908f88b8 Mon Sep 17 00:00:00 2001 From: moritz-suckow Date: Tue, 16 May 2023 14:28:03 +0200 Subject: [PATCH 1/4] Add error message if file param nonexistent #3298 --- .../filter/mergefilter/MergeFilter.kt | 7 ++++++- .../filter/mergefilter/MergeFilterTest.kt | 17 +++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/analysis/filter/MergeFilter/src/main/kotlin/de/maibornwolff/codecharta/filter/mergefilter/MergeFilter.kt b/analysis/filter/MergeFilter/src/main/kotlin/de/maibornwolff/codecharta/filter/mergefilter/MergeFilter.kt index e141832d6d..79a1b23841 100644 --- a/analysis/filter/MergeFilter/src/main/kotlin/de/maibornwolff/codecharta/filter/mergefilter/MergeFilter.kt +++ b/analysis/filter/MergeFilter/src/main/kotlin/de/maibornwolff/codecharta/filter/mergefilter/MergeFilter.kt @@ -59,7 +59,12 @@ class MergeFilter( val sourceFiles = mutableListOf() for (source in sources) { - sourceFiles.addAll(getFilesInFolder(source)) + if (!source.exists()) { + // TODO: Should execution be halted and an error thrown if this happens? Or continue without the file? + logger.error("Could not find file `${ source.path }` and did not merge!") + } else { + sourceFiles.addAll(getFilesInFolder(source)) + } } val srcProjects = sourceFiles diff --git a/analysis/filter/MergeFilter/src/test/kotlin/de/maibornwolff/codecharta/filter/mergefilter/MergeFilterTest.kt b/analysis/filter/MergeFilter/src/test/kotlin/de/maibornwolff/codecharta/filter/mergefilter/MergeFilterTest.kt index 9637f914b6..0dd5880c91 100644 --- a/analysis/filter/MergeFilter/src/test/kotlin/de/maibornwolff/codecharta/filter/mergefilter/MergeFilterTest.kt +++ b/analysis/filter/MergeFilter/src/test/kotlin/de/maibornwolff/codecharta/filter/mergefilter/MergeFilterTest.kt @@ -79,4 +79,21 @@ class MergeFilterTest { assertThat(file.exists()).isTrue } + + @Test + fun `should output warning for all files from parameters which were not found`() { + System.setOut(PrintStream(outContent)) + System.setErr(PrintStream(errContent)) + + CommandLine(MergeFilter()).execute( + "src/test/resources/test.json", "src/test/resources/test2.json", + "src/test/resources/thisDoesNotExist1.json", + "src/test/resources/thisDoesNotExist2.json").toString() + + System.setOut(originalOut) + System.setErr(originalErr) + + assertThat(errContent.toString()).contains("Could not find file `src/test/resources/thisDoesNotExist1.json` and did not merge!") + assertThat(errContent.toString()).contains("Could not find file `src/test/resources/thisDoesNotExist2.json` and did not merge!") + } } From e3d34b537ab15ae0a21821bfd021efe845a14f7e Mon Sep 17 00:00:00 2001 From: moritz-suckow Date: Wed, 17 May 2023 13:36:12 +0200 Subject: [PATCH 2/4] Refactor input check into new class and add Tests #3298 --- .../filter/mergefilter/MergeFilter.kt | 18 ++---- .../filter/mergefilter/MergeFilterTest.kt | 46 ++++++++++---- .../codecharta/util/InputHelper.kt | 35 +++++++++++ .../codecharta/util/InputHelperTest.kt | 62 +++++++++++++++++++ 4 files changed, 137 insertions(+), 24 deletions(-) create mode 100644 analysis/model/src/main/kotlin/de/maibornwolff/codecharta/util/InputHelper.kt create mode 100644 analysis/model/src/test/kotlin/de/maibornwolff/codecharta/util/InputHelperTest.kt diff --git a/analysis/filter/MergeFilter/src/main/kotlin/de/maibornwolff/codecharta/filter/mergefilter/MergeFilter.kt b/analysis/filter/MergeFilter/src/main/kotlin/de/maibornwolff/codecharta/filter/mergefilter/MergeFilter.kt index 79a1b23841..781a36f65b 100644 --- a/analysis/filter/MergeFilter/src/main/kotlin/de/maibornwolff/codecharta/filter/mergefilter/MergeFilter.kt +++ b/analysis/filter/MergeFilter/src/main/kotlin/de/maibornwolff/codecharta/filter/mergefilter/MergeFilter.kt @@ -6,6 +6,7 @@ import de.maibornwolff.codecharta.serialization.ProjectSerializer import de.maibornwolff.codecharta.tools.interactiveparser.InteractiveParser import de.maibornwolff.codecharta.tools.interactiveparser.ParserDialogInterface import de.maibornwolff.codecharta.tools.interactiveparser.util.InteractiveParserHelper +import de.maibornwolff.codecharta.util.InputHelper import mu.KotlinLogging import picocli.CommandLine import java.io.File @@ -57,14 +58,10 @@ class MergeFilter( ) } - val sourceFiles = mutableListOf() - for (source in sources) { - if (!source.exists()) { - // TODO: Should execution be halted and an error thrown if this happens? Or continue without the file? - logger.error("Could not find file `${ source.path }` and did not merge!") - } else { - sourceFiles.addAll(getFilesInFolder(source)) - } + val sourceFiles = InputHelper.getAndCheckAllSpecifiedInputFiles(sources) + if (sourceFiles.isEmpty()) { + logger.error("Aborting execution because one or more input files have not been found!") + return null } val srcProjects = sourceFiles @@ -85,11 +82,6 @@ class MergeFilter( return null } - private fun getFilesInFolder(folder: File): List { - val files = folder.walk().filter { !it.name.startsWith(".") && !it.isDirectory } - return files.toList() - } - companion object { fun mergePipedWithCurrentProject(pipedProject: Project, currentProject: Project): Project { return ProjectMerger( diff --git a/analysis/filter/MergeFilter/src/test/kotlin/de/maibornwolff/codecharta/filter/mergefilter/MergeFilterTest.kt b/analysis/filter/MergeFilter/src/test/kotlin/de/maibornwolff/codecharta/filter/mergefilter/MergeFilterTest.kt index 0dd5880c91..60621bc50d 100644 --- a/analysis/filter/MergeFilter/src/test/kotlin/de/maibornwolff/codecharta/filter/mergefilter/MergeFilterTest.kt +++ b/analysis/filter/MergeFilter/src/test/kotlin/de/maibornwolff/codecharta/filter/mergefilter/MergeFilterTest.kt @@ -1,11 +1,20 @@ package de.maibornwolff.codecharta.filter.mergefilter import de.maibornwolff.codecharta.filter.mergefilter.MergeFilter.Companion.main +import de.maibornwolff.codecharta.model.Project +import de.maibornwolff.codecharta.serialization.ProjectDeserializer +import de.maibornwolff.codecharta.util.InputHelper +import io.mockk.every +import io.mockk.mockkObject +import io.mockk.unmockkAll +import io.mockk.verify import org.assertj.core.api.Assertions.assertThat +import org.junit.jupiter.api.AfterEach import org.junit.jupiter.api.Test import picocli.CommandLine import java.io.ByteArrayOutputStream import java.io.File +import java.io.FileInputStream import java.io.PrintStream class MergeFilterTest { @@ -14,6 +23,11 @@ class MergeFilterTest { val errContent = ByteArrayOutputStream() val originalErr = System.err + @AfterEach + fun unmockEverything() { + unmockkAll() + } + @Test fun `should merge all files in a folder correctly`() { val projectLocation = "src/test/resources/mergeFolderTest" @@ -54,9 +68,12 @@ class MergeFilterTest { @Test fun `should create json uncompressed file`() { + val inputFile1 = "src/test/resources/test.json" + val inputFile2 = "src/test/resources/test2.json" + main( arrayOf( - "src/test/resources/test.json", "src/test/resources/test2.json", "-nc", + inputFile1, inputFile2, "-nc", "-o=src/test/resources/output" ) ) @@ -81,19 +98,26 @@ class MergeFilterTest { } @Test - fun `should output warning for all files from parameters which were not found`() { - System.setOut(PrintStream(outContent)) - System.setErr(PrintStream(errContent)) + fun `should abort if at least one input file does not exist`() { + mockkObject(InputHelper) + every { + InputHelper.getAndCheckAllSpecifiedInputFiles(any()) + } returns mutableListOf() - CommandLine(MergeFilter()).execute( - "src/test/resources/test.json", "src/test/resources/test2.json", - "src/test/resources/thisDoesNotExist1.json", - "src/test/resources/thisDoesNotExist2.json").toString() + mockkObject(ProjectDeserializer) + every { + ProjectDeserializer.deserializeProject(any()) + } returns Project("") - System.setOut(originalOut) + System.setErr(PrintStream(errContent)) + CommandLine(MergeFilter()).execute( + "src/test/resources/mergeFolderTest/file1.cc.json", + "src/test/resources/mergeFolderTest/file2.cc.json", + "src/test/resources/thisDoesNotExist.cc.json").toString() System.setErr(originalErr) - assertThat(errContent.toString()).contains("Could not find file `src/test/resources/thisDoesNotExist1.json` and did not merge!") - assertThat(errContent.toString()).contains("Could not find file `src/test/resources/thisDoesNotExist2.json` and did not merge!") + assertThat(errContent.toString()).contains("Aborting execution because one or more input files have not been found!") + + verify(exactly = 0) { ProjectDeserializer.deserializeProject(any()) } } } diff --git a/analysis/model/src/main/kotlin/de/maibornwolff/codecharta/util/InputHelper.kt b/analysis/model/src/main/kotlin/de/maibornwolff/codecharta/util/InputHelper.kt new file mode 100644 index 0000000000..cbd0d97fde --- /dev/null +++ b/analysis/model/src/main/kotlin/de/maibornwolff/codecharta/util/InputHelper.kt @@ -0,0 +1,35 @@ +package de.maibornwolff.codecharta.util + +import mu.KotlinLogging +import java.io.File + +class InputHelper { + companion object { + private val logger = KotlinLogging.logger {} + + fun getAndCheckAllSpecifiedInputFiles(inputFiles: Array): MutableList { + val resultList = mutableListOf() + var doesInputContainNonexistentFile = false + + for (source in inputFiles) { + if (!source.exists()) { + logger.error("Could not find file `${ source.path }` and did not merge!") + doesInputContainNonexistentFile = true + } else { + resultList.addAll(getFilesInFolder(source)) + } + } + + return if (doesInputContainNonexistentFile) { + mutableListOf() + } else { + resultList + } + } + + private fun getFilesInFolder(folder: File): List { + val files = folder.walk().filter { !it.name.startsWith(".") && !it.isDirectory } + return files.toList() + } + } +} diff --git a/analysis/model/src/test/kotlin/de/maibornwolff/codecharta/util/InputHelperTest.kt b/analysis/model/src/test/kotlin/de/maibornwolff/codecharta/util/InputHelperTest.kt new file mode 100644 index 0000000000..36a30b8f86 --- /dev/null +++ b/analysis/model/src/test/kotlin/de/maibornwolff/codecharta/util/InputHelperTest.kt @@ -0,0 +1,62 @@ +package de.maibornwolff.codecharta.util + +import org.assertj.core.api.Assertions +import org.junit.jupiter.api.Test +import java.io.ByteArrayOutputStream +import java.io.File +import java.io.PrintStream + +class InputHelperTest { + val outContent = ByteArrayOutputStream() + val originalOut = System.out + val errContent = ByteArrayOutputStream() + val originalErr = System.err + + @Test + fun `should output warning for all files from parameters which were not found`() { + System.setOut(PrintStream(outContent)) + System.setErr(PrintStream(errContent)) + + val inputFiles = arrayOf(File("src/test/resources/example.cc.json"), + File("src/test/resources/example_api_version_1.3.cc.json"), + File("src/test/resources/thisDoesNotExist1.json"), + File("src/test/resources/thisDoesNotExist2.json")) + InputHelper.getAndCheckAllSpecifiedInputFiles(inputFiles) + + System.setOut(originalOut) + System.setErr(originalErr) + + Assertions.assertThat(errContent.toString()) + .contains("Could not find file `src/test/resources/thisDoesNotExist1.json` and did not merge!") + Assertions.assertThat(errContent.toString()) + .contains("Could not find file `src/test/resources/thisDoesNotExist2.json` and did not merge!") + } + + @Test + fun `should return empty list if input contains one nonexistent file`() { + val inputFiles = arrayOf(File("src/test/resources/example.cc.json"), + File("src/test/resources/example_api_version_1.3.cc.json"), + File("src/test/resources/thisDoesNotExist1.json")) + + val result = InputHelper.getAndCheckAllSpecifiedInputFiles(inputFiles) + + Assertions.assertThat(result).isEmpty() + } + + @Test + fun `should return list of all input files if all exist`() { + val validFile1 = File("src/test/resources/example.cc.json") + val validFile2 = File("src/test/resources/example_api_version_1.3.cc.json") + val validFile3 = File("src/test/resources/exampleUncompressed.txt") + + val inputFiles = arrayOf(validFile1, validFile2, validFile3) + + val result = InputHelper.getAndCheckAllSpecifiedInputFiles(inputFiles) + + Assertions.assertThat(result).contains(validFile1) + Assertions.assertThat(result).contains(validFile2) + Assertions.assertThat(result).contains(validFile3) + + Assertions.assertThat(result.size).isEqualTo(inputFiles.size) + } +} From d0aa6590eb4558cb68ea2b3dd34082f6783fb46e Mon Sep 17 00:00:00 2001 From: moritz-suckow Date: Wed, 17 May 2023 14:36:44 +0200 Subject: [PATCH 3/4] Add check for empty file input #3298 --- .../codecharta/filter/mergefilter/MergeFilter.kt | 2 +- .../codecharta/filter/mergefilter/MergeFilterTest.kt | 2 +- .../de/maibornwolff/codecharta/util/InputHelper.kt | 5 +++++ .../maibornwolff/codecharta/util/InputHelperTest.kt | 12 ++++++++++++ 4 files changed, 19 insertions(+), 2 deletions(-) diff --git a/analysis/filter/MergeFilter/src/main/kotlin/de/maibornwolff/codecharta/filter/mergefilter/MergeFilter.kt b/analysis/filter/MergeFilter/src/main/kotlin/de/maibornwolff/codecharta/filter/mergefilter/MergeFilter.kt index 781a36f65b..c28407811a 100644 --- a/analysis/filter/MergeFilter/src/main/kotlin/de/maibornwolff/codecharta/filter/mergefilter/MergeFilter.kt +++ b/analysis/filter/MergeFilter/src/main/kotlin/de/maibornwolff/codecharta/filter/mergefilter/MergeFilter.kt @@ -60,7 +60,7 @@ class MergeFilter( val sourceFiles = InputHelper.getAndCheckAllSpecifiedInputFiles(sources) if (sourceFiles.isEmpty()) { - logger.error("Aborting execution because one or more input files have not been found!") + logger.error("Aborting execution because one or more input files have not been found or no input was specified at all!") return null } diff --git a/analysis/filter/MergeFilter/src/test/kotlin/de/maibornwolff/codecharta/filter/mergefilter/MergeFilterTest.kt b/analysis/filter/MergeFilter/src/test/kotlin/de/maibornwolff/codecharta/filter/mergefilter/MergeFilterTest.kt index 60621bc50d..dcda8783f3 100644 --- a/analysis/filter/MergeFilter/src/test/kotlin/de/maibornwolff/codecharta/filter/mergefilter/MergeFilterTest.kt +++ b/analysis/filter/MergeFilter/src/test/kotlin/de/maibornwolff/codecharta/filter/mergefilter/MergeFilterTest.kt @@ -116,7 +116,7 @@ class MergeFilterTest { "src/test/resources/thisDoesNotExist.cc.json").toString() System.setErr(originalErr) - assertThat(errContent.toString()).contains("Aborting execution because one or more input files have not been found!") + assertThat(errContent.toString()).contains("Aborting execution because one or more input files have not been found or no input was specified at all!") verify(exactly = 0) { ProjectDeserializer.deserializeProject(any()) } } diff --git a/analysis/model/src/main/kotlin/de/maibornwolff/codecharta/util/InputHelper.kt b/analysis/model/src/main/kotlin/de/maibornwolff/codecharta/util/InputHelper.kt index cbd0d97fde..b3d107df28 100644 --- a/analysis/model/src/main/kotlin/de/maibornwolff/codecharta/util/InputHelper.kt +++ b/analysis/model/src/main/kotlin/de/maibornwolff/codecharta/util/InputHelper.kt @@ -8,6 +8,11 @@ class InputHelper { private val logger = KotlinLogging.logger {} fun getAndCheckAllSpecifiedInputFiles(inputFiles: Array): MutableList { + if (inputFiles.isEmpty()) { + logger.error("Did not find any input files!") + return mutableListOf() + } + val resultList = mutableListOf() var doesInputContainNonexistentFile = false diff --git a/analysis/model/src/test/kotlin/de/maibornwolff/codecharta/util/InputHelperTest.kt b/analysis/model/src/test/kotlin/de/maibornwolff/codecharta/util/InputHelperTest.kt index 36a30b8f86..e02a654ee2 100644 --- a/analysis/model/src/test/kotlin/de/maibornwolff/codecharta/util/InputHelperTest.kt +++ b/analysis/model/src/test/kotlin/de/maibornwolff/codecharta/util/InputHelperTest.kt @@ -39,8 +39,20 @@ class InputHelperTest { File("src/test/resources/thisDoesNotExist1.json")) val result = InputHelper.getAndCheckAllSpecifiedInputFiles(inputFiles) + Assertions.assertThat(result).isEmpty() + } + + @Test + fun `should return empty list if no files are specified`() { + val inputFiles = arrayOf() + + System.setErr(PrintStream(errContent)) + val result = InputHelper.getAndCheckAllSpecifiedInputFiles(inputFiles) + System.setErr(originalErr) Assertions.assertThat(result).isEmpty() + Assertions.assertThat(errContent.toString()) + .contains("Did not find any input files!") } @Test From 1ddfe862acfc333fdc108cb4f0c5f7a387a1093f Mon Sep 17 00:00:00 2001 From: Phanlezz <65733509+phanlezz@users.noreply.github.com> Date: Fri, 19 May 2023 09:25:50 +0200 Subject: [PATCH 4/4] Fix test for windows based os; Add changelog entry #3298 --- CHANGELOG.md | 1 + .../kotlin/de/maibornwolff/codecharta/util/InputHelperTest.kt | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bbe48958e2..9d18cf4ff4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/) - Show again delta of a building which have nothing in common in red or green [#3271](https://github.com/MaibornWolff/codecharta/pull/3271) - Always show description of suspicious metrics [#3285](https://github.com/MaibornWolff/codecharta/pull/3285) - Show suspicious metrics and risk profile documentation pages in navigation bar [#3290](https://github.com/MaibornWolff/codecharta/pull/3290) +- Merge filter will now abort execution when an invalid file is specified as input [#3305](https://github.com/MaibornWolff/codecharta/pull/3305) ### Changed diff --git a/analysis/model/src/test/kotlin/de/maibornwolff/codecharta/util/InputHelperTest.kt b/analysis/model/src/test/kotlin/de/maibornwolff/codecharta/util/InputHelperTest.kt index e02a654ee2..48a4bb68d0 100644 --- a/analysis/model/src/test/kotlin/de/maibornwolff/codecharta/util/InputHelperTest.kt +++ b/analysis/model/src/test/kotlin/de/maibornwolff/codecharta/util/InputHelperTest.kt @@ -27,9 +27,9 @@ class InputHelperTest { System.setErr(originalErr) Assertions.assertThat(errContent.toString()) - .contains("Could not find file `src/test/resources/thisDoesNotExist1.json` and did not merge!") + .contains("thisDoesNotExist1.json` and did not merge!") Assertions.assertThat(errContent.toString()) - .contains("Could not find file `src/test/resources/thisDoesNotExist2.json` and did not merge!") + .contains("thisDoesNotExist2.json` and did not merge!") } @Test