Merge branch 'master' into cr/22849_duplicate_test_entries

# Conflicts:
#	CHANGELOG.md

CHANGELOG.md

@@ -1,15 +1,19 @@
 We use [semantic versioning](http://semver.org/):
 
 - MAJOR version when you make incompatible API changes,
-- MINOR version when you add functionality in a backwards compatible manner, and
+- MINOR version when you add functionality in a backwards-compatible manner, and
 - PATCH version when you make backwards compatible bug fixes.
 
 # Next Release
 - [fix] Prevent "out of memory" in small JVMs: Don't cache test executions in memory
 - [breaking change] `--ignore-duplicates` (and `-d` option in convert tool) have been replaced with `--duplicates` option
 - [breaking change] `--filter` option in convert tool has been renamed to `--includes`
 - [breaking change] `--exclude` option in convert tool has been renamed to `--excludes`
-- [fix] lower default timeout for http requests and remove retry logic for impacted tests request
+- [fix] Remove retry logic for impacted tests request
+- [fix] Resolve possible memory leak during report generation
+- [feature] Use git.commit.id from git.properties instead of branch and timestamp
+- [breaking change] Reduce XML report size by only including source file coverage, no class coverage
+- [feature] New option `--ignore-uncovered-classes` to further reduce size of XML reports
 - [fix] converter produces duplicate test entries for testwise coverage
 
 # 15.5.0

agent/README.md

@@ -84,6 +84,9 @@ patterns with `*`, `**` and `?`.
 - `duplicates`: defines how JaCoCo handles duplicate class files. This is by default set to `WARN` to make the initial
   setup of the tool as easy as possible. However, this should be set to `FAIL` for productive use if possible. In special 
   cases you can also set it to `IGNORE` to print no warnings. See the special section on `duplicates` below.
+- `ignore-uncovered-classes`: Whether classes without any recorded coverage should be ignored when generating the XML 
+  coverage report. Since Teamscale assumes classes not contained in the report to have no coverage at all, this can 
+  reduce report sizes for large systems (Default is false).
 - `upload-url`: an HTTP(S) URL to which to upload generated XML files. The XML files will be zipped before the upload.
 - `upload-metadata`: paths to files that should also be included in uploaded zips. Separate multiple paths with a 
   semicolon.

agent/src/main/java/com/teamscale/jacoco/agent/Agent.java

@@ -55,7 +55,7 @@ public Agent(AgentOptions options,
 
 		generator = new JaCoCoXmlReportGenerator(options.getClassDirectoriesOrZips(),
 				options.getLocationIncludeFilter(),
-				options.getDuplicateClassFileBehavior(), wrap(logger));
+				options.getDuplicateClassFileBehavior(), options.shouldIgnoreUncoveredClasses(), wrap(logger));
 
 		if (options.shouldDumpInIntervals()) {
 			timer = new Timer(this::dumpReport, Duration.ofMinutes(options.getDumpIntervalInMinutes()));

agent/src/main/java/com/teamscale/jacoco/agent/convert/ConvertCommand.java

@@ -11,6 +11,7 @@
 import com.teamscale.jacoco.agent.commandline.ICommand;
 import com.teamscale.jacoco.agent.commandline.Validator;
 import com.teamscale.report.EDuplicateClassFileBehavior;
+
 import org.conqat.lib.commons.assertion.CCSMAssert;
 import org.conqat.lib.commons.collections.CollectionUtils;
 import org.conqat.lib.commons.filesystem.FileSystemUtils;
@@ -24,7 +25,8 @@
 /**
  * Encapsulates all command line options for the convert command for parsing with {@link JCommander}.
  */
-@Parameters(commandNames = "convert", commandDescription = "Converts a binary .exec coverage file to XML.")
+@Parameters(commandNames = "convert", commandDescription = "Converts a binary .exec coverage file to XML. " +
+		"Note that the XML report will only contain source file coverage information, but no class coverage.")
 public class ConvertCommand implements ICommand {
 
 	/** The directories and/or zips that contain all class files being profiled. */
@@ -72,6 +74,12 @@ public class ConvertCommand implements ICommand {
 			"Options are FAIL, WARN and IGNORE.")
 	/* package */ EDuplicateClassFileBehavior duplicateClassFileBehavior = EDuplicateClassFileBehavior.WARN;
 
+	/** Whether to ignore uncovered class files. */
+	@Parameter(names = {"--ignore-uncovered-classes"}, required = false, arity = 1, description = ""
+			+ "Whether to ignore uncovered classes."
+			+ " These classes will not be part of the XML report at all, making it considerably smaller in some cases. Defaults to false.")
+	/* package */ boolean shouldIgnoreUncoveredClasses = false;
+
 	/** Whether testwise coverage or jacoco coverage should be generated. */
 	@Parameter(names = {"--testwise-coverage", "-t"}, required = false, arity = 0, description = "Whether testwise " +
 			"coverage or jacoco coverage should be generated.")

agent/src/main/java/com/teamscale/jacoco/agent/convert/Converter.java

@@ -16,6 +16,7 @@
 import com.teamscale.report.util.ClasspathWildcardIncludeFilter;
 import com.teamscale.report.util.CommandLineLogger;
 import com.teamscale.report.util.ILogger;
+
 import org.jacoco.core.data.ExecutionDataStore;
 import org.jacoco.core.data.SessionInfo;
 import org.jacoco.core.tools.ExecFileLoader;
@@ -54,7 +55,7 @@ public void runJaCoCoReportGeneration() throws IOException {
 
 		Logger logger = LoggingUtils.getLogger(this);
 		JaCoCoXmlReportGenerator generator = new JaCoCoXmlReportGenerator(arguments.getClassDirectoriesOrZips(),
-				getWildcardIncludeExcludeFilter(), arguments.getDuplicateClassFileBehavior(),
+				getWildcardIncludeExcludeFilter(), arguments.getDuplicateClassFileBehavior(), arguments.shouldIgnoreUncoveredClasses,
 				wrap(logger));
 
 		try (Benchmark benchmark = new Benchmark("Generating the XML report")) {

agent/src/main/java/com/teamscale/jacoco/agent/git_properties/GitPropertiesLocator.java

@@ -1,6 +1,5 @@
 package com.teamscale.jacoco.agent.git_properties;
 
-import com.teamscale.client.CommitDescriptor;
 import com.teamscale.client.StringUtils;
 import com.teamscale.jacoco.agent.upload.delay.DelayedCommitDescriptorUploader;
 import com.teamscale.jacoco.agent.util.DaemonThreadFactory;
@@ -12,9 +11,6 @@
 import java.io.FileInputStream;
 import java.io.IOException;
 import java.nio.file.Paths;
-import java.time.ZonedDateTime;
-import java.time.format.DateTimeFormatter;
-import java.time.format.DateTimeParseException;
 import java.util.Properties;
 import java.util.concurrent.Executor;
 import java.util.concurrent.Executors;
@@ -30,13 +26,9 @@ public class GitPropertiesLocator {
 	/** Name of the git.properties file. */
 	private static final String GIT_PROPERTIES_FILE_NAME = "git.properties";
 
-	/** The standard date format used by git.properties. */
-	private static final DateTimeFormatter GIT_PROPERTIES_DATE_FORMAT = DateTimeFormatter
-			.ofPattern("yyyy-MM-dd'T'HH:mm:ssZ");
-
 	private final Logger logger = LoggingUtils.getLogger(GitPropertiesLocator.class);
 	private final Executor executor;
-	private CommitDescriptor foundCommitDescriptor = null;
+	private String foundRevision = null;
 	private File jarFileWithGitProperties = null;
 
 	private final DelayedCommitDescriptorUploader store;
@@ -66,44 +58,45 @@ public void searchJarFileForGitPropertiesAsync(File jarFile) {
 
 	private void searchJarFile(File jarFile) {
 		try {
-			CommitDescriptor commitDescriptor = getCommitFromGitProperties(jarFile);
-			if (commitDescriptor == null) {
+			String revision = getCommitFromGitProperties(jarFile);
+			if (revision == null) {
 				logger.debug("No git.properties file found in {}", jarFile.toString());
 				return;
 			}
 
-			if (foundCommitDescriptor != null) {
-				if (!foundCommitDescriptor.equals(commitDescriptor)) {
-					logger.error("Found inconsistent git.properties files: {} contained {} while {} contained {}." +
+			if (foundRevision != null) {
+				if (!foundRevision.equals(revision)) {
+					logger.error(
+							"Found inconsistent git.properties files: {} contained SHA1 {} while {} contained {}." +
 									" Please ensure that all git.properties files of your application are consistent." +
 									" Otherwise, you may" +
 									" be uploading to the wrong 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, foundCommitDescriptor, jarFile, commitDescriptor);
+							jarFileWithGitProperties, foundRevision, jarFile, revision);
 				}
 				return;
 			}
 
 			logger.debug("Found git.properties file in {} and found commit descriptor {}", jarFile.toString(),
-					commitDescriptor.toString());
-			foundCommitDescriptor = commitDescriptor;
+					revision);
+			foundRevision = revision;
 			jarFileWithGitProperties = jarFile;
-			store.setCommitAndTriggerAsynchronousUpload(commitDescriptor);
+			store.setCommitAndTriggerAsynchronousUpload(revision);
 		} catch (IOException | InvalidGitPropertiesException e) {
 			logger.error("Error during asynchronous search for git.properties in {}", jarFile.toString(), e);
 		}
 	}
 
 	/**
-	 * Reads branch and timestamp entries from the given jar file's git.properties and builds a commit descriptor out of
-	 * it. If no git.properties file can be found, returns null.
+	 * Reads the git SHA1 from the given jar file's git.properties and builds a commit descriptor out of it. If no
+	 * git.properties file can be found, returns null.
 	 *
 	 * @throws IOException                   If reading the jar file fails.
 	 * @throws InvalidGitPropertiesException If a git.properties file is found but it is malformed.
 	 */
-	public static CommitDescriptor getCommitFromGitProperties(
+	public static String getCommitFromGitProperties(
 			File jarFile) throws IOException, InvalidGitPropertiesException {
 		try (JarInputStream jarStream = new JarInputStream(
 				new BashFileSkippingInputStream(new FileInputStream(jarFile)))) {
@@ -116,7 +109,7 @@ public static CommitDescriptor getCommitFromGitProperties(
 
 	/** Visible for testing. */
 	/*package*/
-	static CommitDescriptor getCommitFromGitProperties(
+	static String getCommitFromGitProperties(
 			JarInputStream jarStream, File jarFile) throws IOException, InvalidGitPropertiesException {
 		JarEntry entry = jarStream.getNextJarEntry();
 		while (entry != null) {
@@ -133,31 +126,16 @@ static CommitDescriptor getCommitFromGitProperties(
 
 	/** Visible for testing. */
 	/*package*/
-	static CommitDescriptor parseGitPropertiesJarEntry(
+	static String parseGitPropertiesJarEntry(
 			String entryName, Properties gitProperties, File jarFile) throws InvalidGitPropertiesException {
-		String branch = gitProperties.getProperty("git.branch");
-		String timestamp = gitProperties.getProperty("git.commit.time");
-
-		if (StringUtils.isEmpty(branch)) {
+		String revision = gitProperties.getProperty("git.commit.id");
+		if (StringUtils.isEmpty(revision)) {
 			throw new InvalidGitPropertiesException(
-					"No entry or empty value for 'git.branch' in " + entryName + " in " + jarFile + "." +
+					"No entry or empty value for 'git.commit.id' in " + entryName + " in " + jarFile + "." +
 							"\nContents of " + GIT_PROPERTIES_FILE_NAME + ": " + gitProperties.toString()
 			);
-		} else if (StringUtils.isEmpty(timestamp)) {
-			throw new InvalidGitPropertiesException(
-					"No entry or empty value for 'git.commit.time' in " + entryName + " in " + jarFile +
-							"\nContents of " + GIT_PROPERTIES_FILE_NAME + ": " + gitProperties.toString());
 		}
 
-		try {
-			long parsedTimestamp = ZonedDateTime.parse(timestamp, GIT_PROPERTIES_DATE_FORMAT).toInstant()
-					.toEpochMilli();
-			branch = StringUtils.stripPrefix(branch, "origin/");
-			return new CommitDescriptor(branch, parsedTimestamp);
-		} catch (DateTimeParseException e) {
-			throw new InvalidGitPropertiesException(
-					"git.commit.time value '" + timestamp + "' in " + entryName + " in " + jarFile +
-							" is malformed: it cannot be parsed with the pattern " + GIT_PROPERTIES_DATE_FORMAT, e);
-		}
+		return revision;
 	}
 }

agent/src/main/java/com/teamscale/jacoco/agent/options/AgentOptions.java

@@ -28,7 +28,7 @@
 import com.teamscale.report.EDuplicateClassFileBehavior;
 import com.teamscale.report.testwise.jacoco.JaCoCoTestwiseReportGenerator;
 import com.teamscale.report.util.ClasspathWildcardIncludeFilter;
-import okhttp3.HttpUrl;
+
 import org.conqat.lib.commons.assertion.CCSMAssert;
 import org.conqat.lib.commons.collections.PairList;
 import org.jacoco.core.runtime.WildcardMatcher;
@@ -49,6 +49,8 @@
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
+import okhttp3.HttpUrl;
+
 /**
  * Parses agent command line options.
  */
@@ -148,11 +150,16 @@ public class AgentOptions {
 	/* package */ Integer httpServerPort = null;
 
 	/**
-	 * How test-wise coverage should be stored handled in test-wise mode.
+	 * How test-wise coverage should be handled in test-wise mode.
 	 */
 	/* package */ ETestWiseCoverageMode testWiseCoverageMode = ETestWiseCoverageMode.EXEC_FILE;
 
 
+	/**
+	 * Whether classes without coverage should be skipped from the XML report.
+	 */
+	/* package */ boolean ignoreUncoveredClasses = false;
+
 	/**
 	 * The configuration necessary to upload files to an azure file storage
 	 */
@@ -199,7 +206,7 @@ public String getOriginalOptionsString() {
 				"You did provide some options prefixed with 'teamscale-', but not all required ones!");
 
 		validator.isTrue(teamscaleServer.revision == null || teamscaleServer.commit == null,
-				"'teamscale-revision' is incompatible with 'teamscale-commit', 'teamscale-commit-manifest-jar', or 'teamscale-git-properties-jar'.");
+				"'teamscale-revision' is incompatible with 'teamscale-commit' and 'teamscale-commit-manifest-jar'.");
 
 		validator.isTrue((azureFileStorageConfig.hasAllRequiredFieldsSet() || azureFileStorageConfig
 						.hasAllRequiredFieldsNull()),
@@ -367,8 +374,8 @@ public IUploader createUploader(Instrumentation instrumentation) throws Uploader
 
 	private IUploader createDelayedTeamscaleUploader(Instrumentation instrumentation) {
 		DelayedCommitDescriptorUploader store = new DelayedCommitDescriptorUploader(
-				commit -> {
-					teamscaleServer.commit = commit;
+				revision -> {
+					teamscaleServer.revision = revision;
 					return new TeamscaleUploader(teamscaleServer);
 				}, outputDirectory);
 		GitPropertiesLocator locator = new GitPropertiesLocator(store);
@@ -468,8 +475,13 @@ public boolean shouldDumpOnExit() {
 		return shouldDumpOnExit;
 	}
 
+	/** @see AgentOptions#testWiseCoverageMode */
 	public ETestWiseCoverageMode getTestWiseCoverageMode() {
 		return testWiseCoverageMode;
 	}
 
+	/** @see #ignoreUncoveredClasses */
+	public boolean shouldIgnoreUncoveredClasses() {
+		return ignoreUncoveredClasses;
+	}
 }

agent/src/main/java/com/teamscale/jacoco/agent/options/AgentOptionsParser.java

@@ -13,7 +13,7 @@
 import com.teamscale.report.EDuplicateClassFileBehavior;
 import com.teamscale.report.util.BashFileSkippingInputStream;
 import com.teamscale.report.util.ILogger;
-import okhttp3.HttpUrl;
+
 import org.conqat.lib.commons.collections.CollectionUtils;
 import org.conqat.lib.commons.collections.Pair;
 import org.conqat.lib.commons.filesystem.AntPatternUtils;
@@ -37,6 +37,8 @@
 import static java.util.stream.Collectors.joining;
 import static java.util.stream.Collectors.toList;
 
+import okhttp3.HttpUrl;
+
 /**
  * Parses agent command line options.
  */
@@ -164,6 +166,8 @@ private boolean handleAgentOptions(AgentOptions options, String key,
 				return true;
 			case "duplicates":
 				options.duplicateClassFileBehavior = EDuplicateClassFileBehavior.valueOf(value.toUpperCase());
+			case "ignore-uncovered-classes":
+				options.ignoreUncoveredClasses = Boolean.parseBoolean(value);
 			case "dump-on-exit":
 				options.shouldDumpOnExit = Boolean.parseBoolean(value);
 				return true;
@@ -243,7 +247,7 @@ private boolean handleTeamscaleOptions(AgentOptions options, String key,
 				options.teamscaleServer.commit = getCommitFromManifest(parseFile(key, value));
 				return true;
 			case "teamscale-git-properties-jar":
-				options.teamscaleServer.commit = parseGitProperties(key, value);
+				options.teamscaleServer.revision = parseGitProperties(key, value);
 				return true;
 			case "teamscale-message":
 				options.teamscaleServer.message = value;
@@ -261,14 +265,14 @@ private boolean handleTeamscaleOptions(AgentOptions options, String key,
 		}
 	}
 
-	private CommitDescriptor parseGitProperties(String key, String value) throws AgentOptionParseException {
+	private String parseGitProperties(String key, String value) throws AgentOptionParseException {
 		File jarFile = parseFile(key, value);
 		try {
-			CommitDescriptor commitDescriptor = GitPropertiesLocator.getCommitFromGitProperties(jarFile);
-			if (commitDescriptor == null) {
+			String commit = GitPropertiesLocator.getCommitFromGitProperties(jarFile);
+			if (commit == null) {
 				throw new AgentOptionParseException("Could not locate a git.properties file in " + jarFile.toString());
 			}
-			return commitDescriptor;
+			return commit;
 		} catch (IOException | InvalidGitPropertiesException e) {
 			throw new AgentOptionParseException("Could not locate a valid git.properties file in " + jarFile.toString(),
 					e);

agent/src/main/java/com/teamscale/jacoco/agent/upload/delay/DelayedCommitDescriptorUploader.java

@@ -23,11 +23,11 @@ public class DelayedCommitDescriptorUploader implements IUploader {
 
 	private final Executor executor;
 	private final Logger logger = LoggingUtils.getLogger(this);
-	private final Function<CommitDescriptor, IUploader> wrappedUploaderFactory;
+	private final Function<String, IUploader> wrappedUploaderFactory;
 	private IUploader wrappedUploader = null;
-	private Path cacheDir;
+	private final Path cacheDir;
 
-	public DelayedCommitDescriptorUploader(Function<CommitDescriptor, IUploader> wrappedUploaderFactory,
+	public DelayedCommitDescriptorUploader(Function<String, IUploader> wrappedUploaderFactory,
 										   Path cacheDir) {
 		this(wrappedUploaderFactory, cacheDir, Executors.newSingleThreadExecutor(
 				new DaemonThreadFactory(DelayedCommitDescriptorUploader.class, "Delayed cache upload thread")));
@@ -37,7 +37,7 @@ public DelayedCommitDescriptorUploader(Function<CommitDescriptor, IUploader> wra
 	 * Visible for testing. Allows tests to control the {@link Executor} to test the asynchronous functionality of this
 	 * class.
 	 */
-	/*package*/ DelayedCommitDescriptorUploader(Function<CommitDescriptor, IUploader> wrappedUploaderFactory,
+	/*package*/ DelayedCommitDescriptorUploader(Function<String, IUploader> wrappedUploaderFactory,
 												Path cacheDir, Executor executor) {
 		this.wrappedUploaderFactory = wrappedUploaderFactory;
 		this.cacheDir = cacheDir;
@@ -83,7 +83,7 @@ public String describe() {
 	 * Sets the commit to upload the XMLs to and asynchronously triggers the upload of all cached XMLs. This method
 	 * should only be called once.
 	 */
-	public synchronized void setCommitAndTriggerAsynchronousUpload(CommitDescriptor commit) {
+	public synchronized void setCommitAndTriggerAsynchronousUpload(String commit) {
 		if (wrappedUploader == null) {
 			wrappedUploader = wrappedUploaderFactory.apply(commit);
 			logger.info("Commit to upload to has been found: {}. Uploading any cached XMLs now to {}", commit,

agent/src/test/java/com/teamscale/jacoco/agent/convert/ConverterTest.java

@@ -33,7 +33,7 @@ public void testSmokeTest(@TempDir File tempDir) throws Exception {
 
 		String xml = FileSystemUtils.readFileUTF8(outputFile);
 		System.err.println(xml);
-		assertThat(xml).isNotEmpty().contains("<class").contains("<counter").contains("TestClass");
+		assertThat(xml).isNotEmpty().contains("<package").contains("<sourcefile").contains("<counter").contains("TestClass");
 	}
 
 	/**