Skip to content

TS-42472 Fixed validation for revision option in testwise mode #690

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
stahlbauer merged 1 commit into from
Apr 10, 2025
Merged

TS-42472 Fixed validation for revision option in testwise mode #690

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ We use [semantic versioning](http://semver.org/):
- [feature] _agent_: Experimental support for Java 25 class files
- [fix] _agent_: Reported wrong version number in docker container
- [fix] _agent_: Erroneous log messages were printed about unmatched probes
- [fix] _agent_: Couldn't receive impacted tests when revision is provided instead of commit

# 35.0.3
- [fix] _teamscale-gradle-plugin_: `IllegalArgumentException`: Can't execute request without discovering it first
Expand Down
19 changes: 12 additions & 7 deletions agent/src/main/java/com/teamscale/jacoco/agent/options/AgentOptions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -377,17 +377,22 @@ private void validateSapNetWeaverConfig(Validator validator) {
}

private void validateTestwiseCoverageConfig(Validator validator) {
validator.isFalse(
!(testwiseCoverageMode == ETestwiseCoverageMode.DISK) && httpServerPort == null,
"You use 'mode' 'TESTWISE' but did use neither 'http-server-port' nor dumping to disk!" +
" One of them is required!");
validator.isTrue(httpServerPort != null,
"You use 'mode=testwise' but did not specify the required option 'http-server-port'!");

validator.isFalse(testwiseCoverageMode == ETestwiseCoverageMode.TEAMSCALE_UPLOAD
&& !teamscaleServer.isConfiguredForSingleProjectTeamscaleUpload(),
validator.isTrue(testwiseCoverageMode != ETestwiseCoverageMode.TEAMSCALE_UPLOAD
|| teamscaleServer.isConfiguredForSingleProjectTeamscaleUpload(),
"You use 'tia-mode=teamscale-upload' but did not set all required 'teamscale-' fields to facilitate" +
" a connection to Teamscale!");
}

validator.isTrue(
testwiseCoverageMode != ETestwiseCoverageMode.TEAMSCALE_UPLOAD || teamscaleServer.hasCommitOrRevision(),
"You use 'tia-mode=teamscale-upload' but did not provide a revision or commit via the agent's '" + TeamscaleConfig.TEAMSCALE_REVISION_OPTION + "', '" +
TeamscaleConfig.TEAMSCALE_REVISION_MANIFEST_JAR_OPTION + "', '" + TeamscaleConfig.TEAMSCALE_COMMIT_OPTION +
"', '" + TeamscaleConfig.TEAMSCALE_COMMIT_MANIFEST_JAR_OPTION + "' or '" +
AgentOptions.GIT_PROPERTIES_JAR_OPTION + "' option." +
" Auto-detecting the git.properties is currently not supported in this mode.");
}

/**
* Creates a {@link TeamscaleClient} based on the agent options. Returns null if the user did not fully configure a
Expand Down
27 changes: 8 additions & 19 deletions agent/src/main/java/com/teamscale/jacoco/agent/testimpact/CoverageToTeamscaleStrategy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,36 +1,25 @@
package com.teamscale.jacoco.agent.testimpact;

import java.io.File;
import java.io.IOException;
import java.util.List;

import org.conqat.lib.commons.filesystem.FileSystemUtils;

import com.teamscale.client.EReportFormat;
import com.teamscale.jacoco.agent.JacocoRuntimeController;
import com.teamscale.jacoco.agent.options.AgentOptions;
import com.teamscale.jacoco.agent.upload.teamscale.TeamscaleConfig;
import com.teamscale.report.testwise.jacoco.JaCoCoTestwiseReportGenerator;
import org.conqat.lib.commons.filesystem.FileSystemUtils;

import java.io.File;
import java.io.IOException;
import java.util.List;

/**
* Strategy that records test-wise coverage and uploads the resulting report to Teamscale. Also handles the
* {@link #testRunStart(List, boolean, boolean, boolean, String, String)} event by retrieving tests to run from Teamscale.
* {@link #testRunStart(List, boolean, boolean, boolean, String, String)} event by retrieving tests to run from
* Teamscale.
*/
public class CoverageToTeamscaleStrategy extends CoverageToJsonStrategyBase {

public CoverageToTeamscaleStrategy(JacocoRuntimeController controller, AgentOptions agentOptions,
JaCoCoTestwiseReportGenerator reportGenerator) {
JaCoCoTestwiseReportGenerator reportGenerator) {
super(controller, agentOptions, reportGenerator);

if (!agentOptions.getTeamscaleServerOptions().hasCommitOrRevision()) {
throw new UnsupportedOperationException(
"You must provide a commit or revision via the agent's '" + TeamscaleConfig.TEAMSCALE_COMMIT_OPTION +
"', '" + TeamscaleConfig.TEAMSCALE_COMMIT_MANIFEST_JAR_OPTION + "', '" +
TeamscaleConfig.TEAMSCALE_REVISION_OPTION + "' or '" +
AgentOptions.GIT_PROPERTIES_JAR_OPTION + "' option." +
" Auto-detecting the git.properties does not work since we need the commit before any code" +
" has been profiled in order to obtain the prioritized test cases from the TIA.");
}
}

@Override
Expand Down
38 changes: 20 additions & 18 deletions agent/src/main/java/com/teamscale/jacoco/agent/testimpact/TestEventHandlerStrategyBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,25 +1,23 @@
package com.teamscale.jacoco.agent.testimpact;

import java.io.IOException;
import java.util.Collections;
import java.util.List;

import org.slf4j.Logger;

import com.teamscale.client.ClusteredTestDetails;
import com.teamscale.client.HttpUtils;
import com.teamscale.client.PrioritizableTestCluster;
import com.teamscale.client.TeamscaleClient;
import com.teamscale.jacoco.agent.JacocoRuntimeController;
import com.teamscale.jacoco.agent.logging.LoggingUtils;
import com.teamscale.jacoco.agent.options.AgentOptions;
import com.teamscale.jacoco.agent.upload.teamscale.TeamscaleConfig;
import com.teamscale.jacoco.agent.logging.LoggingUtils;
import com.teamscale.report.testwise.jacoco.cache.CoverageGenerationException;
import com.teamscale.report.testwise.model.TestExecution;
import com.teamscale.report.testwise.model.TestInfo;

import org.slf4j.Logger;
import retrofit2.Response;

import java.io.IOException;
import java.util.Collections;
import java.util.List;

/** Base class for strategies to handle test events. */
public abstract class TestEventHandlerStrategyBase {

Expand Down Expand Up @@ -63,7 +61,7 @@ public void testStart(String test) {
* as a json response.
*/
public TestInfo testEnd(String test,
TestExecution testExecution) throws JacocoRuntimeController.DumpException, CoverageGenerationException {
TestExecution testExecution) throws JacocoRuntimeController.DumpException, CoverageGenerationException {
if (testExecution != null) {
testExecution.uniformPath = test;
if (startTimestamp != -1) {
Expand All @@ -86,9 +84,9 @@ public TestInfo testEnd(String test,
* @throws UnsupportedOperationException if the user did not properly configure the {@link #teamscaleClient}.
*/
public List<PrioritizableTestCluster> testRunStart(List<ClusteredTestDetails> availableTests,
boolean includeNonImpactedTests,
boolean includeAddedTests, boolean includeFailedAndSkipped,
String baseline, String baselineRevision) throws IOException {
boolean includeNonImpactedTests,
boolean includeAddedTests, boolean includeFailedAndSkipped,
String baseline, String baselineRevision) throws IOException {
int availableTestCount = 0;
if (availableTests != null) {
availableTestCount = availableTests.size();
Expand All @@ -99,13 +97,14 @@ public List<PrioritizableTestCluster> testRunStart(List<ClusteredTestDetails> av

Response<List<PrioritizableTestCluster>> response = teamscaleClient
.getImpactedTests(availableTests, baseline, baselineRevision,
agentOptions.getTeamscaleServerOptions().commit, agentOptions.getTeamscaleServerOptions().revision,
agentOptions.getTeamscaleServerOptions().commit,
agentOptions.getTeamscaleServerOptions().revision,
agentOptions.getTeamscaleServerOptions().repository,
Collections.singletonList(agentOptions.getTeamscaleServerOptions().partition),
includeNonImpactedTests, includeAddedTests, includeFailedAndSkipped);
if (response.isSuccessful()) {
List<PrioritizableTestCluster> prioritizableTestClusters = response.body();
logger.debug("Teamscale suggested these tests: {}", prioritizableTestClusters.toString());
logger.debug("Teamscale suggested these tests: {}", prioritizableTestClusters);
return prioritizableTestClusters;
} else {
String responseBody = HttpUtils.getErrorBodyStringSafe(response);
Expand All @@ -121,11 +120,14 @@ private void validateConfiguration() {
" Thus, you cannot use the agent to retrieve impacted tests via the testrun/start REST endpoint." +
" Please use the 'teamscale-' agent parameters to configure a Teamscale connection.");
}
if (agentOptions.getTeamscaleServerOptions().commit == null) {
if (!agentOptions.getTeamscaleServerOptions().hasCommitOrRevision()) {
throw new UnsupportedOperationException(
"You did not provide a '" + TeamscaleConfig.TEAMSCALE_COMMIT_OPTION + "' or '" +
TeamscaleConfig.TEAMSCALE_COMMIT_MANIFEST_JAR_OPTION + "'. '" +
TeamscaleConfig.TEAMSCALE_REVISION_OPTION + "' is not sufficient to retrieve impacted tests.");
"You must provide a revision or commit via the agent's '" + TeamscaleConfig.TEAMSCALE_REVISION_OPTION + "', '" +
TeamscaleConfig.TEAMSCALE_REVISION_MANIFEST_JAR_OPTION + "', '" + TeamscaleConfig.TEAMSCALE_COMMIT_OPTION +
"', '" + TeamscaleConfig.TEAMSCALE_COMMIT_MANIFEST_JAR_OPTION + "' or '" +
AgentOptions.GIT_PROPERTIES_JAR_OPTION + "' option." +
" Auto-detecting the git.properties does not work since we need the commit before any code" +
" has been profiled in order to obtain the prioritized test cases from the TIA.");
}
}

Expand Down
27 changes: 24 additions & 3 deletions agent/src/test/java/com/teamscale/jacoco/agent/options/AgentOptionsParserTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public void testUploadMethodRecognition() throws Exception {
assertThat(parseAndThrow(basicTeamscaleOptions + ",teamscale-project=proj")
.determineUploadMethod()).isEqualTo(AgentOptions.EUploadMethod.TEAMSCALE_SINGLE_PROJECT);
assertThat(parseAndThrow(
basicTeamscaleOptions + ",sap-nwdi-applications=com.package.MyClass:projectId;com.company.Main:project")
basicTeamscaleOptions + ",sap-nwdi-applications=com.package.MyClass:projectId;com.company.Main:project")
.determineUploadMethod())
.isEqualTo(AgentOptions.EUploadMethod.SAP_NWDI_TEAMSCALE);
}
Expand All @@ -89,7 +89,7 @@ public void testUploadMethodRecognitionWithTeamscaleProperties() throws Exceptio
assertThat(parseAndThrow(basicTeamscaleOptions + ",teamscale-project=proj")
.determineUploadMethod()).isEqualTo(AgentOptions.EUploadMethod.TEAMSCALE_SINGLE_PROJECT);
assertThat(parseAndThrow(
basicTeamscaleOptions + ",sap-nwdi-applications=com.package.MyClass:projectId;com.company.Main:project")
basicTeamscaleOptions + ",sap-nwdi-applications=com.package.MyClass:projectId;com.company.Main:project")
.determineUploadMethod())
.isEqualTo(AgentOptions.EUploadMethod.SAP_NWDI_TEAMSCALE);
}
Expand Down Expand Up @@ -220,6 +220,26 @@ public void revisionOrCommitRequireProject() {
).hasMessageContaining("you did not provide the 'teamscale-project'");
}

@Test
public void testTeamscaleUploadRequiresRevisionOrCommit() {
String teamscaleBaseOptions = "teamscale-server-url=teamscale.com,teamscale-user=user,teamscale-access-token=token,teamscale-partition=p,teamscale-project=proj,mode=testwise,http-server-port=8123,";
assertThatThrownBy(
() -> parseAndThrow(
teamscaleBaseOptions + "tia-mode=teamscale-upload")
).hasMessageContaining("You use 'tia-mode=teamscale-upload' but did not provide a revision or commit");

assertThatCode(
() -> parseAndThrow(
teamscaleBaseOptions + "tia-mode=teamscale-upload,teamscale-revision=12345")
).doesNotThrowAnyException();

assertThatCode(
() -> parseAndThrow(
teamscaleBaseOptions + "tia-mode=teamscale-upload,teamscale-commit=master:HEAD")
).doesNotThrowAnyException();
}


@Test
public void environmentConfigIdDoesNotExist() {
mockWebServer.enqueue(new MockResponse().setResponseCode(404).setBody("invalid-config-id does not exist"));
Expand All @@ -245,7 +265,8 @@ public void mustPreserveDefaultExcludes() throws Exception {

@Test
public void teamscalePropertiesCredentialsUsedAsDefaultButOverridable() throws Exception {
assertThat(parseAndThrow(new AgentOptionsParser(new CommandLineLogger(), null, null, teamscaleCredentials), "teamscale-project=p,teamscale-partition=p").teamscaleServer.userName).isEqualTo(
assertThat(parseAndThrow(new AgentOptionsParser(new CommandLineLogger(), null, null, teamscaleCredentials),
"teamscale-project=p,teamscale-partition=p").teamscaleServer.userName).isEqualTo(
"user");
assertThat(parseAndThrow(new AgentOptionsParser(new CommandLineLogger(), null, null, teamscaleCredentials),
"teamscale-project=p,teamscale-partition=p,teamscale-user=user2").teamscaleServer.userName).isEqualTo(
Expand Down
60 changes: 33 additions & 27 deletions .../src/test/java/com/teamscale/jacoco/agent/testimpact/CoverageToTeamscaleStrategyTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,24 +1,5 @@
package com.teamscale.jacoco.agent.testimpact;

import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.ArgumentMatchers.matches;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import java.io.File;
import java.io.IOException;
import java.util.Collections;
import java.util.List;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.jupiter.api.io.TempDir;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;

import com.teamscale.client.ClusteredTestDetails;
import com.teamscale.client.CommitDescriptor;
import com.teamscale.client.EReportFormat;
Expand All @@ -34,10 +15,29 @@
import com.teamscale.report.testwise.model.TestwiseCoverage;
import com.teamscale.report.testwise.model.builder.FileCoverageBuilder;
import com.teamscale.report.testwise.model.builder.TestCoverageBuilder;

import okhttp3.HttpUrl;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.jupiter.api.io.TempDir;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import retrofit2.Response;

import java.io.File;
import java.io.IOException;
import java.util.Collections;
import java.util.List;

import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyBoolean;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.ArgumentMatchers.matches;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

@ExtendWith(MockitoExtension.class)
public class CoverageToTeamscaleStrategyTest {

Expand All @@ -55,7 +55,7 @@ public class CoverageToTeamscaleStrategyTest {

@Test
public void shouldRecordCoverageForTestsEvenIfNotProvidedAsAvailableTest() throws Exception {
AgentOptions options = mockOptions();
AgentOptions options = mockOptions(false);
CoverageToTeamscaleStrategy strategy = new CoverageToTeamscaleStrategy(controller, options, reportGenerator);

TestwiseCoverage testwiseCoverage = getDummyTestwiseCoverage("mytest");
Expand All @@ -71,18 +71,20 @@ public void shouldRecordCoverageForTestsEvenIfNotProvidedAsAvailableTest() throw
any(), any(), any(), any(), any());
}

@Test
public void testValidCallSequence() throws Exception {
@ParameterizedTest
@ValueSource(booleans = {true, false})
public void testValidCallSequence(boolean useRevision) throws Exception {
List<PrioritizableTestCluster> clusters = Collections
.singletonList(new PrioritizableTestCluster("cluster",
Collections.singletonList(new PrioritizableTest("mytest"))));
when(client.getImpactedTests(any(), any(), any(), any(), any(), any(), any(), anyBoolean(), anyBoolean(), anyBoolean())).thenReturn(
when(client.getImpactedTests(any(), any(), any(), any(), any(), any(), any(), anyBoolean(), anyBoolean(),
anyBoolean())).thenReturn(
Response.success(clusters));

TestwiseCoverage testwiseCoverage = getDummyTestwiseCoverage("mytest");
when(reportGenerator.convert(any(File.class))).thenReturn(testwiseCoverage);

AgentOptions options = mockOptions();
AgentOptions options = mockOptions(useRevision);
JacocoRuntimeController controller = mock(JacocoRuntimeController.class);
CoverageToTeamscaleStrategy strategy = new CoverageToTeamscaleStrategy(controller, options, reportGenerator);

Expand Down Expand Up @@ -111,13 +113,17 @@ protected static TestwiseCoverage getDummyTestwiseCoverage(String test) {
return testwiseCoverage;
}

private AgentOptions mockOptions() throws IOException {
private AgentOptions mockOptions(boolean useRevision) throws IOException {
AgentOptions options = mock(AgentOptions.class);
when(options.createTeamscaleClient(true)).thenReturn(client);
when(options.createNewFileInOutputDirectory(any(), any())).thenReturn(new File(tempDir, "test"));

TeamscaleServer server = new TeamscaleServer();
server.commit = new CommitDescriptor("branch", "12345");
if (useRevision) {
server.revision = "rev1";
} else {
server.commit = new CommitDescriptor("branch", "12345");
}
server.url = HttpUrl.get("http://doesnt-exist.io");
server.userName = "build";
server.userAccessToken = "token";
Expand Down
Loading