Skip to content

Commit 0807850

Browse files
DreierFDreierF
committed
TS-42472 Fixed validation for revision option in testwise mode
1 parent 50a19f5 commit 0807850

6 files changed

Lines changed: 98 additions & 74 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ We use [semantic versioning](http://semver.org/):
88
- [feature] _agent_: Experimental support for Java 25 class files
99
- [fix] _agent_: Reported wrong version number in docker container
1010
- [fix] _agent_: Erroneous log messages were printed about unmatched probes
11+
- [fix] _agent_: Couldn't receive impacted tests when revision is provided instead of commit
1112

1213
# 35.0.3
1314
- [fix] _teamscale-gradle-plugin_: `IllegalArgumentException`: Can't execute request without discovering it first

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

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -377,17 +377,22 @@ private void validateSapNetWeaverConfig(Validator validator) {
377377
}
378378

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

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

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

392397
/**
393398
* Creates a {@link TeamscaleClient} based on the agent options. Returns null if the user did not fully configure a

‎agent/src/main/java/com/teamscale/jacoco/agent/testimpact/CoverageToTeamscaleStrategy.java‎

Lines changed: 8 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,36 +1,25 @@
11
package com.teamscale.jacoco.agent.testimpact;
22

3-
import java.io.File;
4-
import java.io.IOException;
5-
import java.util.List;
6-
7-
import org.conqat.lib.commons.filesystem.FileSystemUtils;
8-
93
import com.teamscale.client.EReportFormat;
104
import com.teamscale.jacoco.agent.JacocoRuntimeController;
115
import com.teamscale.jacoco.agent.options.AgentOptions;
12-
import com.teamscale.jacoco.agent.upload.teamscale.TeamscaleConfig;
136
import com.teamscale.report.testwise.jacoco.JaCoCoTestwiseReportGenerator;
7+
import org.conqat.lib.commons.filesystem.FileSystemUtils;
8+
9+
import java.io.File;
10+
import java.io.IOException;
11+
import java.util.List;
1412

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

2120
public CoverageToTeamscaleStrategy(JacocoRuntimeController controller, AgentOptions agentOptions,
22-
JaCoCoTestwiseReportGenerator reportGenerator) {
21+
JaCoCoTestwiseReportGenerator reportGenerator) {
2322
super(controller, agentOptions, reportGenerator);
24-
25-
if (!agentOptions.getTeamscaleServerOptions().hasCommitOrRevision()) {
26-
throw new UnsupportedOperationException(
27-
"You must provide a commit or revision via the agent's '" + TeamscaleConfig.TEAMSCALE_COMMIT_OPTION +
28-
"', '" + TeamscaleConfig.TEAMSCALE_COMMIT_MANIFEST_JAR_OPTION + "', '" +
29-
TeamscaleConfig.TEAMSCALE_REVISION_OPTION + "' or '" +
30-
AgentOptions.GIT_PROPERTIES_JAR_OPTION + "' option." +
31-
" Auto-detecting the git.properties does not work since we need the commit before any code" +
32-
" has been profiled in order to obtain the prioritized test cases from the TIA.");
33-
}
3423
}
3524

3625
@Override

‎agent/src/main/java/com/teamscale/jacoco/agent/testimpact/TestEventHandlerStrategyBase.java‎

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,23 @@
11
package com.teamscale.jacoco.agent.testimpact;
22

3-
import java.io.IOException;
4-
import java.util.Collections;
5-
import java.util.List;
6-
7-
import org.slf4j.Logger;
8-
93
import com.teamscale.client.ClusteredTestDetails;
104
import com.teamscale.client.HttpUtils;
115
import com.teamscale.client.PrioritizableTestCluster;
126
import com.teamscale.client.TeamscaleClient;
137
import com.teamscale.jacoco.agent.JacocoRuntimeController;
8+
import com.teamscale.jacoco.agent.logging.LoggingUtils;
149
import com.teamscale.jacoco.agent.options.AgentOptions;
1510
import com.teamscale.jacoco.agent.upload.teamscale.TeamscaleConfig;
16-
import com.teamscale.jacoco.agent.logging.LoggingUtils;
1711
import com.teamscale.report.testwise.jacoco.cache.CoverageGenerationException;
1812
import com.teamscale.report.testwise.model.TestExecution;
1913
import com.teamscale.report.testwise.model.TestInfo;
20-
14+
import org.slf4j.Logger;
2115
import retrofit2.Response;
2216

17+
import java.io.IOException;
18+
import java.util.Collections;
19+
import java.util.List;
20+
2321
/** Base class for strategies to handle test events. */
2422
public abstract class TestEventHandlerStrategyBase {
2523

@@ -63,7 +61,7 @@ public void testStart(String test) {
6361
* as a json response.
6462
*/
6563
public TestInfo testEnd(String test,
66-
TestExecution testExecution) throws JacocoRuntimeController.DumpException, CoverageGenerationException {
64+
TestExecution testExecution) throws JacocoRuntimeController.DumpException, CoverageGenerationException {
6765
if (testExecution != null) {
6866
testExecution.uniformPath = test;
6967
if (startTimestamp != -1) {
@@ -86,9 +84,9 @@ public TestInfo testEnd(String test,
8684
* @throws UnsupportedOperationException if the user did not properly configure the {@link #teamscaleClient}.
8785
*/
8886
public List<PrioritizableTestCluster> testRunStart(List<ClusteredTestDetails> availableTests,
89-
boolean includeNonImpactedTests,
90-
boolean includeAddedTests, boolean includeFailedAndSkipped,
91-
String baseline, String baselineRevision) throws IOException {
87+
boolean includeNonImpactedTests,
88+
boolean includeAddedTests, boolean includeFailedAndSkipped,
89+
String baseline, String baselineRevision) throws IOException {
9290
int availableTestCount = 0;
9391
if (availableTests != null) {
9492
availableTestCount = availableTests.size();
@@ -99,13 +97,14 @@ public List<PrioritizableTestCluster> testRunStart(List<ClusteredTestDetails> av
9997

10098
Response<List<PrioritizableTestCluster>> response = teamscaleClient
10199
.getImpactedTests(availableTests, baseline, baselineRevision,
102-
agentOptions.getTeamscaleServerOptions().commit, agentOptions.getTeamscaleServerOptions().revision,
100+
agentOptions.getTeamscaleServerOptions().commit,
101+
agentOptions.getTeamscaleServerOptions().revision,
103102
agentOptions.getTeamscaleServerOptions().repository,
104103
Collections.singletonList(agentOptions.getTeamscaleServerOptions().partition),
105104
includeNonImpactedTests, includeAddedTests, includeFailedAndSkipped);
106105
if (response.isSuccessful()) {
107106
List<PrioritizableTestCluster> prioritizableTestClusters = response.body();
108-
logger.debug("Teamscale suggested these tests: {}", prioritizableTestClusters.toString());
107+
logger.debug("Teamscale suggested these tests: {}", prioritizableTestClusters);
109108
return prioritizableTestClusters;
110109
} else {
111110
String responseBody = HttpUtils.getErrorBodyStringSafe(response);
@@ -121,11 +120,14 @@ private void validateConfiguration() {
121120
" Thus, you cannot use the agent to retrieve impacted tests via the testrun/start REST endpoint." +
122121
" Please use the 'teamscale-' agent parameters to configure a Teamscale connection.");
123122
}
124-
if (agentOptions.getTeamscaleServerOptions().commit == null) {
123+
if (!agentOptions.getTeamscaleServerOptions().hasCommitOrRevision()) {
125124
throw new UnsupportedOperationException(
126-
"You did not provide a '" + TeamscaleConfig.TEAMSCALE_COMMIT_OPTION + "' or '" +
127-
TeamscaleConfig.TEAMSCALE_COMMIT_MANIFEST_JAR_OPTION + "'. '" +
128-
TeamscaleConfig.TEAMSCALE_REVISION_OPTION + "' is not sufficient to retrieve impacted tests.");
125+
"You must provide a revision or commit via the agent's '" + TeamscaleConfig.TEAMSCALE_REVISION_OPTION + "', '" +
126+
TeamscaleConfig.TEAMSCALE_REVISION_MANIFEST_JAR_OPTION + "', '" + TeamscaleConfig.TEAMSCALE_COMMIT_OPTION +
127+
"', '" + TeamscaleConfig.TEAMSCALE_COMMIT_MANIFEST_JAR_OPTION + "' or '" +
128+
AgentOptions.GIT_PROPERTIES_JAR_OPTION + "' option." +
129+
" Auto-detecting the git.properties does not work since we need the commit before any code" +
130+
" has been profiled in order to obtain the prioritized test cases from the TIA.");
129131
}
130132
}
131133

‎agent/src/test/java/com/teamscale/jacoco/agent/options/AgentOptionsParserTest.java‎

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ public void testUploadMethodRecognition() throws Exception {
6464
assertThat(parseAndThrow(basicTeamscaleOptions + ",teamscale-project=proj")
6565
.determineUploadMethod()).isEqualTo(AgentOptions.EUploadMethod.TEAMSCALE_SINGLE_PROJECT);
6666
assertThat(parseAndThrow(
67-
basicTeamscaleOptions + ",sap-nwdi-applications=com.package.MyClass:projectId;com.company.Main:project")
67+
basicTeamscaleOptions + ",sap-nwdi-applications=com.package.MyClass:projectId;com.company.Main:project")
6868
.determineUploadMethod())
6969
.isEqualTo(AgentOptions.EUploadMethod.SAP_NWDI_TEAMSCALE);
7070
}
@@ -89,7 +89,7 @@ public void testUploadMethodRecognitionWithTeamscaleProperties() throws Exceptio
8989
assertThat(parseAndThrow(basicTeamscaleOptions + ",teamscale-project=proj")
9090
.determineUploadMethod()).isEqualTo(AgentOptions.EUploadMethod.TEAMSCALE_SINGLE_PROJECT);
9191
assertThat(parseAndThrow(
92-
basicTeamscaleOptions + ",sap-nwdi-applications=com.package.MyClass:projectId;com.company.Main:project")
92+
basicTeamscaleOptions + ",sap-nwdi-applications=com.package.MyClass:projectId;com.company.Main:project")
9393
.determineUploadMethod())
9494
.isEqualTo(AgentOptions.EUploadMethod.SAP_NWDI_TEAMSCALE);
9595
}
@@ -220,6 +220,26 @@ public void revisionOrCommitRequireProject() {
220220
).hasMessageContaining("you did not provide the 'teamscale-project'");
221221
}
222222

223+
@Test
224+
public void testTeamscaleUploadRequiresRevisionOrCommit() {
225+
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,";
226+
assertThatThrownBy(
227+
() -> parseAndThrow(
228+
teamscaleBaseOptions + "tia-mode=teamscale-upload")
229+
).hasMessageContaining("You use 'tia-mode=teamscale-upload' but did not provide a revision or commit");
230+
231+
assertThatCode(
232+
() -> parseAndThrow(
233+
teamscaleBaseOptions + "tia-mode=teamscale-upload,teamscale-revision=12345")
234+
).doesNotThrowAnyException();
235+
236+
assertThatCode(
237+
() -> parseAndThrow(
238+
teamscaleBaseOptions + "tia-mode=teamscale-upload,teamscale-commit=master:HEAD")
239+
).doesNotThrowAnyException();
240+
}
241+
242+
223243
@Test
224244
public void environmentConfigIdDoesNotExist() {
225245
mockWebServer.enqueue(new MockResponse().setResponseCode(404).setBody("invalid-config-id does not exist"));
@@ -245,7 +265,8 @@ public void mustPreserveDefaultExcludes() throws Exception {
245265

246266
@Test
247267
public void teamscalePropertiesCredentialsUsedAsDefaultButOverridable() throws Exception {
248-
assertThat(parseAndThrow(new AgentOptionsParser(new CommandLineLogger(), null, null, teamscaleCredentials), "teamscale-project=p,teamscale-partition=p").teamscaleServer.userName).isEqualTo(
268+
assertThat(parseAndThrow(new AgentOptionsParser(new CommandLineLogger(), null, null, teamscaleCredentials),
269+
"teamscale-project=p,teamscale-partition=p").teamscaleServer.userName).isEqualTo(
249270
"user");
250271
assertThat(parseAndThrow(new AgentOptionsParser(new CommandLineLogger(), null, null, teamscaleCredentials),
251272
"teamscale-project=p,teamscale-partition=p,teamscale-user=user2").teamscaleServer.userName).isEqualTo(

‎agent/src/test/java/com/teamscale/jacoco/agent/testimpact/CoverageToTeamscaleStrategyTest.java‎

Lines changed: 33 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,5 @@
11
package com.teamscale.jacoco.agent.testimpact;
22

3-
import static org.mockito.ArgumentMatchers.any;
4-
import static org.mockito.ArgumentMatchers.anyBoolean;
5-
import static org.mockito.ArgumentMatchers.eq;
6-
import static org.mockito.ArgumentMatchers.matches;
7-
import static org.mockito.Mockito.mock;
8-
import static org.mockito.Mockito.verify;
9-
import static org.mockito.Mockito.when;
10-
11-
import java.io.File;
12-
import java.io.IOException;
13-
import java.util.Collections;
14-
import java.util.List;
15-
16-
import org.junit.jupiter.api.Test;
17-
import org.junit.jupiter.api.extension.ExtendWith;
18-
import org.junit.jupiter.api.io.TempDir;
19-
import org.mockito.Mock;
20-
import org.mockito.junit.jupiter.MockitoExtension;
21-
223
import com.teamscale.client.ClusteredTestDetails;
234
import com.teamscale.client.CommitDescriptor;
245
import com.teamscale.client.EReportFormat;
@@ -34,10 +15,29 @@
3415
import com.teamscale.report.testwise.model.TestwiseCoverage;
3516
import com.teamscale.report.testwise.model.builder.FileCoverageBuilder;
3617
import com.teamscale.report.testwise.model.builder.TestCoverageBuilder;
37-
3818
import okhttp3.HttpUrl;
19+
import org.junit.jupiter.api.Test;
20+
import org.junit.jupiter.api.extension.ExtendWith;
21+
import org.junit.jupiter.api.io.TempDir;
22+
import org.junit.jupiter.params.ParameterizedTest;
23+
import org.junit.jupiter.params.provider.ValueSource;
24+
import org.mockito.Mock;
25+
import org.mockito.junit.jupiter.MockitoExtension;
3926
import retrofit2.Response;
4027

28+
import java.io.File;
29+
import java.io.IOException;
30+
import java.util.Collections;
31+
import java.util.List;
32+
33+
import static org.mockito.ArgumentMatchers.any;
34+
import static org.mockito.ArgumentMatchers.anyBoolean;
35+
import static org.mockito.ArgumentMatchers.eq;
36+
import static org.mockito.ArgumentMatchers.matches;
37+
import static org.mockito.Mockito.mock;
38+
import static org.mockito.Mockito.verify;
39+
import static org.mockito.Mockito.when;
40+
4141
@ExtendWith(MockitoExtension.class)
4242
public class CoverageToTeamscaleStrategyTest {
4343

@@ -55,7 +55,7 @@ public class CoverageToTeamscaleStrategyTest {
5555

5656
@Test
5757
public void shouldRecordCoverageForTestsEvenIfNotProvidedAsAvailableTest() throws Exception {
58-
AgentOptions options = mockOptions();
58+
AgentOptions options = mockOptions(false);
5959
CoverageToTeamscaleStrategy strategy = new CoverageToTeamscaleStrategy(controller, options, reportGenerator);
6060

6161
TestwiseCoverage testwiseCoverage = getDummyTestwiseCoverage("mytest");
@@ -71,18 +71,20 @@ public void shouldRecordCoverageForTestsEvenIfNotProvidedAsAvailableTest() throw
7171
any(), any(), any(), any(), any());
7272
}
7373

74-
@Test
75-
public void testValidCallSequence() throws Exception {
74+
@ParameterizedTest
75+
@ValueSource(booleans = {true, false})
76+
public void testValidCallSequence(boolean useRevision) throws Exception {
7677
List<PrioritizableTestCluster> clusters = Collections
7778
.singletonList(new PrioritizableTestCluster("cluster",
7879
Collections.singletonList(new PrioritizableTest("mytest"))));
79-
when(client.getImpactedTests(any(), any(), any(), any(), any(), any(), any(), anyBoolean(), anyBoolean(), anyBoolean())).thenReturn(
80+
when(client.getImpactedTests(any(), any(), any(), any(), any(), any(), any(), anyBoolean(), anyBoolean(),
81+
anyBoolean())).thenReturn(
8082
Response.success(clusters));
8183

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

85-
AgentOptions options = mockOptions();
87+
AgentOptions options = mockOptions(useRevision);
8688
JacocoRuntimeController controller = mock(JacocoRuntimeController.class);
8789
CoverageToTeamscaleStrategy strategy = new CoverageToTeamscaleStrategy(controller, options, reportGenerator);
8890

@@ -111,13 +113,17 @@ protected static TestwiseCoverage getDummyTestwiseCoverage(String test) {
111113
return testwiseCoverage;
112114
}
113115

114-
private AgentOptions mockOptions() throws IOException {
116+
private AgentOptions mockOptions(boolean useRevision) throws IOException {
115117
AgentOptions options = mock(AgentOptions.class);
116118
when(options.createTeamscaleClient(true)).thenReturn(client);
117119
when(options.createNewFileInOutputDirectory(any(), any())).thenReturn(new File(tempDir, "test"));
118120

119121
TeamscaleServer server = new TeamscaleServer();
120-
server.commit = new CommitDescriptor("branch", "12345");
122+
if (useRevision) {
123+
server.revision = "rev1";
124+
} else {
125+
server.commit = new CommitDescriptor("branch", "12345");
126+
}
121127
server.url = HttpUrl.get("http://doesnt-exist.io");
122128
server.userName = "build";
123129
server.userAccessToken = "token";

0 commit comments

Comments
 (0)