From 1ff9ad79b24c4f5a2908b00b6e06ad67f94bd5b3 Mon Sep 17 00:00:00 2001 From: Fabian Streitel Date: Thu, 20 Aug 2026 14:33:27 +0200 Subject: [PATCH] TS-47508 Accept a folder for the git-properties-jar option The option only accepted a regular file so far. A folder was rejected with a warning and the option was dropped, so deployments that keep git.properties in a resources folder instead of inside the profiled archive got no commit auto-detection at all. The search itself already handles both shapes: GitPropertiesLocatorUtils.findGitPropertiesInFile branches into an archive or a folder search, and the folder branch is what the automatic detection uses for exploded class folders such as BOOT-INF/classes or the JBoss vfs temp folders. The only obstacles were the parser's isFile() check and the three call sites hardcoding isJarFile = true, which now derive the flag from isDirectory(). Folder semantics stay as they are: the folder tree is always walked for git.properties files, while search-git-properties-recursively only controls whether archives inside the folder are opened as well. Changing that would also change the automatic detection for exploded class folders, where git.properties usually sits in a subfolder. The option keeps its name for compatibility, so an option named "...-jar" now also takes a folder. Log messages and documentation state this. Also fixes the inconsistency warning in GitSingleProjectPropertiesLocator, which pointed users to teamscale-git-properties-jar. That option was replaced by git-properties-jar in 31.0.0. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 1 + .../GitMultiProjectPropertiesLocator.kt | 6 ++-- .../GitSingleProjectPropertiesLocator.kt | 19 +++++++------ .../jacoco/agent/options/AgentOptions.kt | 21 ++++++++------ .../agent/options/AgentOptionsParser.kt | 21 +++++++------- .../upload/artifactory/ArtifactoryConfig.kt | 28 ++++++++++++------- .../jacoco/agent/options/AgentOptionsTest.kt | 8 +++--- .../DelayedCommitDescriptorRetrievalTest.kt | 20 +++++++++++-- .../git-properties-folder/git.properties | 3 ++ 9 files changed, 79 insertions(+), 48 deletions(-) create mode 100644 agent/src/test/resources/com/teamscale/jacoco/agent/upload/delay/git-properties-folder/git.properties diff --git a/CHANGELOG.md b/CHANGELOG.md index dc8b99dcc..39f92a69b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ We use [semantic versioning](http://semver.org/): - PATCH version when you make backwards compatible bug fixes. # Next version +- [feature] _agent_: `git-properties-jar` now also accepts a folder, which is searched for `git.properties` files. Previously, a folder was rejected with a warning and no commit was auto-detected. - [breaking] _agent_: Log lines now show the simple class name (`INFO Agent - ...`) instead of the fully qualified one. - [breaking] _agent_: In testwise coverage mode, the test result is now mandatory in the body of `/test/end` requests. Requests without a result (or without a body at all) are rejected with "400 Bad Request" instead of silently producing a report entry without a result. The duration remains optional: if it is omitted, the profiler derives it from the time between the `/test/start` and `/test/end` requests. The profiler additionally logs a warning if no `/test/start` request was received, since the duration cannot be derived reliably in that case. - [breaking] _tia-client_: Removed the `ITestwiseCoverageAgentApi.testFinished(testUniformPath)` overload, which sent a `/test/end` request without a body. Since the agent now rejects such requests, use `testFinished(testUniformPath, testExecution)` instead. diff --git a/agent/src/main/kotlin/com/teamscale/jacoco/agent/commit_resolution/git_properties/GitMultiProjectPropertiesLocator.kt b/agent/src/main/kotlin/com/teamscale/jacoco/agent/commit_resolution/git_properties/GitMultiProjectPropertiesLocator.kt index 411c08111..abbe15e7c 100644 --- a/agent/src/main/kotlin/com/teamscale/jacoco/agent/commit_resolution/git_properties/GitMultiProjectPropertiesLocator.kt +++ b/agent/src/main/kotlin/com/teamscale/jacoco/agent/commit_resolution/git_properties/GitMultiProjectPropertiesLocator.kt @@ -11,7 +11,7 @@ import java.util.concurrent.Executor import java.util.concurrent.Executors /** - * Searches a Jar/War/Ear/... file for a git.properties file in order to enable upload for the commit described therein, + * Searches a Jar/War/Ear/... file or a folder for a git.properties file in order to enable upload for the commit described therein, * e.g. to Teamscale, via a [DelayedTeamscaleMultiProjectUploader]. Specifically, this searches for the * 'teamscale.project' property specified in each of the discovered 'git.properties' files. */ @@ -37,7 +37,7 @@ class GitMultiProjectPropertiesLocator( ) /** - * Asynchronously searches the given jar file for git.properties files and adds a corresponding uploader to the + * Asynchronously searches the given jar file or folder for git.properties files and adds a corresponding uploader to the * multi-project uploader. */ override fun searchFileForGitPropertiesAsync(file: File, isJarFile: Boolean) { @@ -45,7 +45,7 @@ class GitMultiProjectPropertiesLocator( } /** - * Synchronously searches the given jar file for git.properties files and adds a corresponding uploader to the + * Synchronously searches the given jar file or folder for git.properties files and adds a corresponding uploader to the * multi-project uploader. */ @VisibleForTesting diff --git a/agent/src/main/kotlin/com/teamscale/jacoco/agent/commit_resolution/git_properties/GitSingleProjectPropertiesLocator.kt b/agent/src/main/kotlin/com/teamscale/jacoco/agent/commit_resolution/git_properties/GitSingleProjectPropertiesLocator.kt index 4217974af..683dc390a 100644 --- a/agent/src/main/kotlin/com/teamscale/jacoco/agent/commit_resolution/git_properties/GitSingleProjectPropertiesLocator.kt +++ b/agent/src/main/kotlin/com/teamscale/jacoco/agent/commit_resolution/git_properties/GitSingleProjectPropertiesLocator.kt @@ -1,6 +1,7 @@ package com.teamscale.jacoco.agent.commit_resolution.git_properties import com.teamscale.jacoco.agent.logging.LoggingUtils.getLogger +import com.teamscale.jacoco.agent.options.AgentOptions import com.teamscale.jacoco.agent.upload.delay.DelayedUploader import com.teamscale.jacoco.agent.util.DaemonThreadFactory import java.io.File @@ -10,8 +11,8 @@ import java.util.concurrent.Executor import java.util.concurrent.Executors /** - * Searches a Jar/War/Ear/... file for a git.properties file in order to enable upload for the commit described therein, - * e.g. to Teamscale, via a [DelayedUploader]. + * Searches a Jar/War/Ear/... file or a folder for a git.properties file in order to enable upload for the commit + * described therein, e.g. to Teamscale, via a [DelayedUploader]. */ class GitSingleProjectPropertiesLocator( private val uploader: DelayedUploader, @@ -27,17 +28,17 @@ class GitSingleProjectPropertiesLocator( ) : IGitPropertiesLocator { private val logger = getLogger(this) private var foundData: T? = null - private var jarFileWithGitProperties: File? = null + private var fileWithGitProperties: File? = null /** - * Asynchronously searches the given jar file for a git.properties file. + * Asynchronously searches the given jar file or folder for a git.properties file. */ override fun searchFileForGitPropertiesAsync(file: File, isJarFile: Boolean) { executor.execute { searchFile(file, isJarFile) } } private fun searchFile(file: File, isJarFile: Boolean) { - logger.debug("Searching jar file {} for a single git.properties", file) + logger.debug("Searching {} for a single git.properties", file) try { val data = dataExtractor.extractData(file, isJarFile, recursiveSearch, gitPropertiesCommitTimeFormat) if (data.isEmpty()) { @@ -60,9 +61,9 @@ class GitSingleProjectPropertiesLocator( " Otherwise, you may" + " be uploading to the wrong project/commit which will result in incorrect coverage data" + " displayed in Teamscale. If you cannot fix the inconsistency, you can manually" + - " specify a Jar/War/Ear/... file from which to read the correct git.properties" + - " file with the agent's teamscale-git-properties-jar parameter.", - jarFileWithGitProperties, foundData, file, data + " specify a Jar/War/Ear/... file or folder from which to read the correct git.properties" + + " file with the agent's ${AgentOptions.GIT_PROPERTIES_JAR_OPTION} parameter.", + fileWithGitProperties, foundData, file, data ) } return @@ -73,7 +74,7 @@ class GitSingleProjectPropertiesLocator( dataEntry ) foundData = dataEntry - jarFileWithGitProperties = file + fileWithGitProperties = file uploader.setCommitAndTriggerAsynchronousUpload(dataEntry) } catch (e: IOException) { logger.error( diff --git a/agent/src/main/kotlin/com/teamscale/jacoco/agent/options/AgentOptions.kt b/agent/src/main/kotlin/com/teamscale/jacoco/agent/options/AgentOptions.kt index 89342e3b1..38b514a81 100644 --- a/agent/src/main/kotlin/com/teamscale/jacoco/agent/options/AgentOptions.kt +++ b/agent/src/main/kotlin/com/teamscale/jacoco/agent/options/AgentOptions.kt @@ -56,7 +56,7 @@ import kotlin.math.max * Parses agent command line options. */ open class AgentOptions(private val logger: ILogger) { - /** See [GIT_PROPERTIES_JAR_OPTION] */ + /** See [GIT_PROPERTIES_JAR_OPTION]. May be an archive or a folder. */ @JvmField var gitPropertiesJar: File? = null @@ -446,7 +446,7 @@ open class AgentOptions(private val logger: ILogger) { jar.absolutePath + " for a git.properties file." ) artifactoryConfig.commitInfo = ArtifactoryConfig.parseGitProperties( - jar, searchGitPropertiesRecursively, gitPropertiesCommitTimeFormat + jar, !jar.isDirectory(), searchGitPropertiesRecursively, gitPropertiesCommitTimeFormat ) } if (!artifactoryConfig.hasCommitInfo()) { @@ -472,7 +472,7 @@ open class AgentOptions(private val logger: ILogger) { "auto-detect it by searching the provided " + GIT_PROPERTIES_JAR_OPTION + " at " + jar.absolutePath + " for a git.properties file." ) - startGitPropertiesSearchInJarFile(uploader, jar) + startGitPropertiesSearch(uploader, jar) return uploader } @@ -503,7 +503,7 @@ open class AgentOptions(private val logger: ILogger) { " auto-detect it by searching the provided " + GIT_PROPERTIES_JAR_OPTION + " at " + jar.absolutePath + " for a git.properties file." ) - startMultiGitPropertiesFileSearchInJarFile(uploader, jar) + startMultiGitPropertiesFileSearch(uploader, jar) return uploader } logger.info( @@ -515,7 +515,7 @@ open class AgentOptions(private val logger: ILogger) { return uploader } - private fun startGitPropertiesSearchInJarFile( + private fun startGitPropertiesSearch( uploader: DelayedUploader, gitPropertiesJar: File ) { @@ -523,7 +523,7 @@ open class AgentOptions(private val logger: ILogger) { uploader, searchGitPropertiesRecursively, gitPropertiesCommitTimeFormat ) { file, isJarFile, recursiveSearch, timeFormat -> getProjectRevisionsFromGitProperties(file, isJarFile, recursiveSearch, timeFormat) - }.searchFileForGitPropertiesAsync(gitPropertiesJar, true) + }.searchFileForGitPropertiesAsync(gitPropertiesJar, !gitPropertiesJar.isDirectory()) } private fun registerSingleGitPropertiesLocator( @@ -556,13 +556,13 @@ open class AgentOptions(private val logger: ILogger) { TeamscaleUploader(teamscaleServer, reportFormat) } - private fun startMultiGitPropertiesFileSearchInJarFile( + private fun startMultiGitPropertiesFileSearch( uploader: DelayedTeamscaleMultiProjectUploader, gitPropertiesJar: File ) { GitMultiProjectPropertiesLocator( uploader, searchGitPropertiesRecursively, gitPropertiesCommitTimeFormat - ).searchFileForGitPropertiesAsync(gitPropertiesJar, true) + ).searchFileForGitPropertiesAsync(gitPropertiesJar, !gitPropertiesJar.isDirectory()) } private fun registerMultiGitPropertiesLocator( @@ -718,7 +718,10 @@ open class AgentOptions(private val logger: ILogger) { const val DEFAULT_EXCLUDES = "kotlin.*:shadow.*:com.sun.*:sun.*:org.eclipse.*:org.junit.*:junit.*:org.apache.*:org.slf4j.*:javax.*:org.gradle.*:java.*:org.jboss.*:org.wildfly.*:org.springframework.*:com.fasterxml.*:jakarta.*:org.aspectj.*:org.h2.*:org.hibernate.*:org.assertj.*:org.mockito.*:org.thymeleaf.*" - /** Option name that allows to specify a jar file that contains the git commit hash in a git.properties file. */ + /** + * Option name that allows to specify a jar/war/ear/aar file or a folder that contains the git commit hash in a + * git.properties file. + */ const val GIT_PROPERTIES_JAR_OPTION = "git-properties-jar" /** diff --git a/agent/src/main/kotlin/com/teamscale/jacoco/agent/options/AgentOptionsParser.kt b/agent/src/main/kotlin/com/teamscale/jacoco/agent/options/AgentOptionsParser.kt index d48859a57..4ce9f1841 100644 --- a/agent/src/main/kotlin/com/teamscale/jacoco/agent/options/AgentOptionsParser.kt +++ b/agent/src/main/kotlin/com/teamscale/jacoco/agent/options/AgentOptionsParser.kt @@ -291,11 +291,11 @@ class AgentOptionsParser @VisibleForTesting internal constructor( "The option " + ArtifactoryConfig.ARTIFACTORY_GIT_PROPERTIES_JAR_OPTION + " is deprecated. It still has an effect, " + "but should be replaced with the equivalent option " + AgentOptions.GIT_PROPERTIES_JAR_OPTION + "." ) - options.gitPropertiesJar = getGitPropertiesJarFile(value) + options.gitPropertiesJar = getGitPropertiesFileOrFolder(value) return true } AgentOptions.GIT_PROPERTIES_JAR_OPTION -> { - options.gitPropertiesJar = getGitPropertiesJarFile(value) + options.gitPropertiesJar = getGitPropertiesFileOrFolder(value) return true } ArtifactoryConfig.ARTIFACTORY_GIT_PROPERTIES_COMMIT_DATE_FORMAT_OPTION -> { @@ -393,17 +393,18 @@ class AgentOptionsParser @VisibleForTesting internal constructor( readConfigFromString(options, configuration.profilerConfiguration!!.configurationOptions) } - private fun getGitPropertiesJarFile(path: String): File? { - val jarFile = File(path) - if (!jarFile.exists()) { + /** + * Resolves the path given for [AgentOptions.GIT_PROPERTIES_JAR_OPTION]. Both an archive (jar/war/ear/aar) and a + * folder are accepted, since the git.properties search handles either. A path that does not exist is not treated + * as a fatal error, as the profiled application should keep running in that case. + */ + private fun getGitPropertiesFileOrFolder(path: String): File? { + val gitPropertiesSearchRoot = File(path) + if (!gitPropertiesSearchRoot.exists()) { logger.warn("The path provided with the ${AgentOptions.GIT_PROPERTIES_JAR_OPTION} option does not exist: $path. Continuing without searching it for git.properties files.") return null } - if (!jarFile.isFile()) { - logger.warn("The path provided with the ${AgentOptions.GIT_PROPERTIES_JAR_OPTION} option is not a regular file (probably a folder instead): $path. Continuing without searching it for git.properties files.") - return null - } - return jarFile + return gitPropertiesSearchRoot } /** diff --git a/agent/src/main/kotlin/com/teamscale/jacoco/agent/upload/artifactory/ArtifactoryConfig.kt b/agent/src/main/kotlin/com/teamscale/jacoco/agent/upload/artifactory/ArtifactoryConfig.kt index 5a180c07a..ad0f96855 100644 --- a/agent/src/main/kotlin/com/teamscale/jacoco/agent/upload/artifactory/ArtifactoryConfig.kt +++ b/agent/src/main/kotlin/com/teamscale/jacoco/agent/upload/artifactory/ArtifactoryConfig.kt @@ -4,6 +4,7 @@ import com.teamscale.jacoco.agent.commit_resolution.git_properties.CommitInfo import com.teamscale.jacoco.agent.commit_resolution.git_properties.GitPropertiesLocatorUtils import com.teamscale.jacoco.agent.commit_resolution.git_properties.InvalidGitPropertiesException import com.teamscale.jacoco.agent.options.AgentOptionParseException +import com.teamscale.jacoco.agent.options.AgentOptions import com.teamscale.jacoco.agent.options.AgentOptionsParser import com.teamscale.jacoco.agent.upload.UploaderException import com.teamscale.jacoco.agent.upload.artifactory.ArtifactoryConfig.Companion.ARTIFACTORY_API_KEY_OPTION @@ -171,37 +172,44 @@ class ArtifactoryConfig { } } - /** Parses the commit information form a git.properties file. */ + /** + * Parses the commit information from a git.properties file. The search root is either an archive + * (isJarFile = true) or a folder. + */ @Throws(UploaderException::class) fun parseGitProperties( - jarFile: File, searchRecursively: Boolean, gitPropertiesCommitTimeFormat: DateTimeFormatter? + gitPropertiesSearchRoot: File, + isJarFile: Boolean, + searchRecursively: Boolean, + gitPropertiesCommitTimeFormat: DateTimeFormatter? ): CommitInfo? { try { val commitInfo = GitPropertiesLocatorUtils.getCommitInfoFromGitProperties( - jarFile, - true, + gitPropertiesSearchRoot, + isJarFile, searchRecursively, gitPropertiesCommitTimeFormat ) if (commitInfo.isEmpty()) { throw UploaderException( - "Found no git.properties files in $jarFile." + - " The 'artifactory-git-properties-jar' option must point to a JAR that contains" + - " a git.properties with the commit from which the JAR was built." + "Found no git.properties files in $gitPropertiesSearchRoot." + + " The '${AgentOptions.GIT_PROPERTIES_JAR_OPTION}' option must point to a" + + " Jar/War/Ear/Aar file or a folder that contains a git.properties with the commit from" + + " which the profiled code was built." ) } if (commitInfo.size > 1) { throw UploaderException( - ("Found multiple git.properties files in " + jarFile + ("Found multiple git.properties files in " + gitPropertiesSearchRoot + ". Uploading to multiple projects is currently not possible with Artifactory. " + "Please contact CQSE if you need this feature.") ) } return commitInfo.firstOrNull() } catch (e: IOException) { - throw UploaderException("Could not locate a valid git.properties file in $jarFile", e) + throw UploaderException("Could not locate a valid git.properties file in $gitPropertiesSearchRoot", e) } catch (e: InvalidGitPropertiesException) { - throw UploaderException("Could not locate a valid git.properties file in $jarFile", e) + throw UploaderException("Could not locate a valid git.properties file in $gitPropertiesSearchRoot", e) } } } diff --git a/agent/src/test/kotlin/com/teamscale/jacoco/agent/options/AgentOptionsTest.kt b/agent/src/test/kotlin/com/teamscale/jacoco/agent/options/AgentOptionsTest.kt index 084bb3b25..2aa5545f8 100644 --- a/agent/src/test/kotlin/com/teamscale/jacoco/agent/options/AgentOptionsTest.kt +++ b/agent/src/test/kotlin/com/teamscale/jacoco/agent/options/AgentOptionsTest.kt @@ -267,17 +267,17 @@ class AgentOptionsTest { } /** - * Test that agent continues to run if the user provided a folder via the - * [AgentOptions.GIT_PROPERTIES_JAR_OPTION] jar option. + * Test that a folder is accepted for the [AgentOptions.GIT_PROPERTIES_JAR_OPTION] option, since the git.properties + * search handles folders as well as archives. */ @Test @Throws(Exception::class) - fun testGitPropertiesJarDoesNotAcceptFolders() { + fun testGitPropertiesJarAcceptsFolders() { val jarFile = File(javaClass.getResource("nested-jar.war")!!.file) val agentOptions = parseAndMaybeThrow( "${AgentOptions.GIT_PROPERTIES_JAR_OPTION}=${jarFile.getParent()}" ) - Assertions.assertThat(agentOptions.gitPropertiesJar).isNull() + Assertions.assertThat(agentOptions.gitPropertiesJar).isDirectory() } /** Tests that supplying version info is supported in Testwise mode. */ diff --git a/agent/src/test/kotlin/com/teamscale/jacoco/agent/upload/delay/DelayedCommitDescriptorRetrievalTest.kt b/agent/src/test/kotlin/com/teamscale/jacoco/agent/upload/delay/DelayedCommitDescriptorRetrievalTest.kt index cc7e13b0d..1e71b176e 100644 --- a/agent/src/test/kotlin/com/teamscale/jacoco/agent/upload/delay/DelayedCommitDescriptorRetrievalTest.kt +++ b/agent/src/test/kotlin/com/teamscale/jacoco/agent/upload/delay/DelayedCommitDescriptorRetrievalTest.kt @@ -20,6 +20,20 @@ class DelayedCommitDescriptorRetrievalTest { @Test @Throws(Exception::class) fun locatorShouldTriggerUploadOfCachedXmls(@TempDir outputPath: Path) { + assertLocatorTriggersUpload( + outputPath, File(javaClass.getResource("git-properties.jar")!!.toURI()), isJarFile = true + ) + } + + @Test + @Throws(Exception::class) + fun locatorShouldTriggerUploadOfCachedXmlsForFolder(@TempDir outputPath: Path) { + assertLocatorTriggersUpload( + outputPath, File(javaClass.getResource("git-properties-folder")!!.toURI()), isJarFile = false + ) + } + + private fun assertLocatorTriggersUpload(outputPath: Path, gitPropertiesSearchRoot: File, isJarFile: Boolean) { val storeExecutor = Executors.newSingleThreadExecutor() val coverageFilePath = outputPath .resolve(String.format("jacoco-%d.xml", ZonedDateTime.now().toInstant().toEpochMilli())) @@ -31,14 +45,14 @@ class DelayedCommitDescriptorRetrievalTest { val locatorExecutor = Executors.newSingleThreadExecutor() val locator = GitSingleProjectPropertiesLocator( store, true, null, locatorExecutor - ) { file, isJarFile, recursiveSearch, timeFormatter -> + ) { file, isArchive, recursiveSearch, timeFormatter -> getCommitInfoFromGitProperties( - file, isJarFile, recursiveSearch, timeFormatter + file, isArchive, recursiveSearch, timeFormatter ) } store.upload(coverageFile) - locator.searchFileForGitPropertiesAsync(File(javaClass.getResource("git-properties.jar")!!.toURI()), true) + locator.searchFileForGitPropertiesAsync(gitPropertiesSearchRoot, isJarFile) locatorExecutor.shutdown() locatorExecutor.awaitTermination(5, TimeUnit.SECONDS) storeExecutor.shutdown() diff --git a/agent/src/test/resources/com/teamscale/jacoco/agent/upload/delay/git-properties-folder/git.properties b/agent/src/test/resources/com/teamscale/jacoco/agent/upload/delay/git-properties-folder/git.properties new file mode 100644 index 000000000..ca6e2cf50 --- /dev/null +++ b/agent/src/test/resources/com/teamscale/jacoco/agent/upload/delay/git-properties-folder/git.properties @@ -0,0 +1,3 @@ +git.commit.id=72c7b3f7e6c4802414283cdf7622e6127f3f8976 +git.branch=master +git.commit.time=2022-02-24T15:43:23+0100