Skip to content

Commit

Permalink
TS-36682 Replaced Moshi with Jackson
Browse files Browse the repository at this point in the history
  • Loading branch information
DreierF committed Nov 30, 2023
1 parent b820284 commit 790df3f
Show file tree
Hide file tree
Showing 38 changed files with 254 additions and 235 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ We use [semantic versioning](http://semver.org/):

# Next Release
- [feature] _teamscale-maven-plugin_: Added the `upload-coverage` goal which automatically uploads JaCoCo reports to Teamscale.
- [fix] _agent_: The agent crashed while starting on some machines with "cannot construct instances of" errors

# 32.4.2
- [fix] _agent_: The agent crashed while starting on some machines with "cannot construct instances of" errors
Expand Down
4 changes: 2 additions & 2 deletions agent/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,10 @@ dependencies {

implementation(libs.retrofit.core)

implementation(libs.moshi)
implementation(libs.jackson.databind)

testImplementation(project(":tia-client"))
testImplementation(libs.retrofit.converter.moshi)
testImplementation(libs.retrofit.converter.jackson)
testImplementation(libs.okhttp.mockwebserver)
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
package com.teamscale.jacoco.agent.testimpact;

import com.squareup.moshi.JsonAdapter;
import com.squareup.moshi.Moshi;
import com.teamscale.client.ClusteredTestDetails;
import com.teamscale.client.PrioritizableTestCluster;
import com.teamscale.jacoco.agent.JacocoRuntimeController;
Expand All @@ -15,6 +13,7 @@
import com.teamscale.report.testwise.model.TestwiseCoverageReport;
import com.teamscale.report.testwise.model.builder.TestCoverageBuilder;
import com.teamscale.report.testwise.model.builder.TestwiseCoverageReportBuilder;
import com.teamscale.report.util.JsonUtils;
import org.slf4j.Logger;

import java.io.File;
Expand All @@ -25,10 +24,9 @@
import static java.util.stream.Collectors.toList;

/**
* Base for strategies that produce testwise coverage information
* in JSON and store or send this data further.
* Base for strategies that produce testwise coverage information in JSON and store or send this data further.
*/
public abstract class CoverageToJsonStrategyBase extends TestEventHandlerStrategyBase{
public abstract class CoverageToJsonStrategyBase extends TestEventHandlerStrategyBase {

/**
* The logger to use.
Expand All @@ -43,13 +41,10 @@ public abstract class CoverageToJsonStrategyBase extends TestEventHandlerStrateg
private final List<TestExecution> testExecutions = new ArrayList<>();
private List<ClusteredTestDetails> availableTests = new ArrayList<>();

private final JsonAdapter<TestwiseCoverageReport> testwiseCoverageReportJsonAdapter = new Moshi.Builder().build()
.adapter(TestwiseCoverageReport.class);

private final JaCoCoTestwiseReportGenerator reportGenerator;

public CoverageToJsonStrategyBase(JacocoRuntimeController controller, AgentOptions agentOptions,
JaCoCoTestwiseReportGenerator reportGenerator) {
JaCoCoTestwiseReportGenerator reportGenerator) {
super(agentOptions, controller);
this.reportGenerator = reportGenerator;
}
Expand Down Expand Up @@ -146,7 +141,7 @@ private String createTestwiseCoverageReport(boolean partial) throws IOException,
availableTests.clear();
testExecutions.clear();

return testwiseCoverageReportJsonAdapter.toJson(report);
return JsonUtils.serialize(report);
}

}
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
package com.teamscale.jacoco.agent.testimpact;

import com.squareup.moshi.JsonAdapter;
import com.squareup.moshi.Moshi;
import com.teamscale.jacoco.agent.util.LoggingUtils;
import com.teamscale.report.testwise.model.TestExecution;
import com.teamscale.report.util.JsonUtils;
import org.slf4j.Logger;

import java.io.File;
Expand All @@ -20,9 +19,6 @@ public class TestExecutionWriter {

private final Logger logger = LoggingUtils.getLogger(this);

private final JsonAdapter<TestExecution> testExecutionAdapter = new Moshi.Builder().build()
.adapter(TestExecution.class);

private final File testExecutionFile;
private boolean hasWrittenAtLeastOneExecution = false;

Expand All @@ -33,7 +29,7 @@ public TestExecutionWriter(File testExecutionFile) {

/** Appends the given {@link TestExecution} to the test execution list file. */
public synchronized void append(TestExecution testExecution) throws IOException {
String json = testExecutionAdapter.toJson(testExecution);
String json = JsonUtils.serialize(testExecution);

// the file contains a JSON array if it exists and to append to it, we strip the trailing "]" and append
// our new entry and a closing "]"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,12 @@ public void testTestwiseCoverageSmokeTest(@TempDir File tempDir) throws Exceptio
String json = FileSystemUtils.readFileUTF8(new File(tempDir, "testwise-coverage-1.json"));
assertThat(json).
contains(
"\"uniformPath\": \"[engine:junit-vintage]/[runner:org.conqat.lib.cqddl.CQDDLTest]/[test:testFunctions(org.conqat.lib.cqddl.CQDDLTest)]\"")
"\"uniformPath\" : \"[engine:junit-vintage]/[runner:org.conqat.lib.cqddl.CQDDLTest]/[test:testFunctions(org.conqat.lib.cqddl.CQDDLTest)]\"")
.contains(
"\"uniformPath\": \"[engine:junit-vintage]/[runner:org.conqat.lib.cqddl.CQDDLTest]/[test:testDirectObjectInsertion(org.conqat.lib.cqddl.CQDDLTest)]\"")
.contains("\"uniformPath\": \"[engine:junit-vintage]/[runner:org.conqat.lib.cqddl.CQDDLTest]/[test:testKeyAbbreviations(org.conqat.lib.cqddl.CQDDLTest)]\"")
.contains("\"uniformPath\": \"[engine:junit-vintage]/[runner:org.conqat.lib.cqddl.CQDDLTest]/[test:testKeyAbbreviations(org.conqat.lib.cqddl.CQDDLTest)]\"")
.contains("\"result\": \"PASSED\"").contains("\"duration\": 1234").contains("\"coveredLines\": \"33,46-47");
"\"uniformPath\" : \"[engine:junit-vintage]/[runner:org.conqat.lib.cqddl.CQDDLTest]/[test:testDirectObjectInsertion(org.conqat.lib.cqddl.CQDDLTest)]\"")
.contains("\"uniformPath\" : \"[engine:junit-vintage]/[runner:org.conqat.lib.cqddl.CQDDLTest]/[test:testKeyAbbreviations(org.conqat.lib.cqddl.CQDDLTest)]\"")
.contains("\"uniformPath\" : \"[engine:junit-vintage]/[runner:org.conqat.lib.cqddl.CQDDLTest]/[test:testKeyAbbreviations(org.conqat.lib.cqddl.CQDDLTest)]\"")
.contains("\"result\" : \"PASSED\"").contains("\"duration\" : 1234").contains("\"coveredLines\" : \"33,46-47");
}

private void copyResourceTo(String name, File targetDir) throws URISyntaxException, IOException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public void shouldRecordCoverageForTestsEvenIfNotProvidedAsAvailableTest() throw
strategy.testRunEnd(false);

verify(client).uploadReport(eq(EReportFormat.TESTWISE_COVERAGE),
matches("\\Q{\"partial\":false,\"tests\":[{\"duration\":\\E[^,]*\\Q,\"paths\":[{\"files\":[{\"coveredLines\":\"1-4\",\"fileName\":\"Main.java\"}],\"path\":\"src/main/java\"}],\"result\":\"PASSED\",\"sourcePath\":\"mytest\",\"uniformPath\":\"mytest\"}]}\\E"),
matches("\\Q{\"partial\":false,\"tests\":[{\"uniformPath\":\"mytest\",\"sourcePath\":\"mytest\",\"duration\":\\E[^,]*\\Q,\"result\":\"PASSED\",\"paths\":[{\"path\":\"src/main/java\",\"files\":[{\"fileName\":\"Main.java\",\"coveredLines\":\"1-4\"}]}]}]}\\E"),
any(), any(), any(), any());
}

Expand Down Expand Up @@ -94,7 +94,7 @@ public void testValidCallSequence() throws Exception {
strategy.testRunEnd(true);

verify(client).uploadReport(eq(EReportFormat.TESTWISE_COVERAGE),
matches("\\Q{\"partial\":true,\"tests\":[{\"content\":\"content\",\"duration\":\\E[^,]*\\Q,\"paths\":[{\"files\":[{\"coveredLines\":\"1-4\",\"fileName\":\"Main.java\"}],\"path\":\"src/main/java\"}],\"result\":\"PASSED\",\"sourcePath\":\"mytest\",\"uniformPath\":\"mytest\"}]}\\E"),
matches("\\Q{\"partial\":true,\"tests\":[{\"uniformPath\":\"mytest\",\"sourcePath\":\"mytest\",\"content\":\"content\",\"duration\":\\E[^,]*\\Q,\"result\":\"PASSED\",\"paths\":[{\"path\":\"src/main/java\",\"files\":[{\"fileName\":\"Main.java\",\"coveredLines\":\"1-4\"}]}]}]}\\E"),
any(), any(), any(), any());
}

Expand Down Expand Up @@ -126,4 +126,4 @@ private AgentOptions mockOptions() throws IOException {
return options;
}

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public void testOneExecution(@TempDir Path tempDir) throws Exception {
TestExecutionWriter writer = new TestExecutionWriter(tempFile.toFile());
writer.append(new TestExecution("test1", 123, ETestExecutionResult.PASSED));
String json = String.join("\n", Files.readAllLines(tempFile));
assertThat(json).isEqualTo("[{\"durationMillis\":123,\"result\":\"PASSED\",\"uniformPath\":\"test1\"}]");
assertThat(json).isEqualTo("[{\"uniformPath\":\"test1\",\"durationMillis\":123,\"result\":\"PASSED\"}]");
}

@Test
Expand All @@ -29,9 +29,9 @@ public void testMultipleExecutions(@TempDir Path tempDir) throws Exception {
writer.append(new TestExecution("test2", 123, ETestExecutionResult.PASSED));
writer.append(new TestExecution("test3", 123, ETestExecutionResult.PASSED));
String json = String.join("\n", Files.readAllLines(tempFile));
assertThat(json).isEqualTo("[{\"durationMillis\":123,\"result\":\"PASSED\",\"uniformPath\":\"test1\"}" +
",{\"durationMillis\":123,\"result\":\"PASSED\",\"uniformPath\":\"test2\"}" +
",{\"durationMillis\":123,\"result\":\"PASSED\",\"uniformPath\":\"test3\"}]");
assertThat(json).isEqualTo("[{\"uniformPath\":\"test1\",\"durationMillis\":123,\"result\":\"PASSED\"}" +
",{\"uniformPath\":\"test2\",\"durationMillis\":123,\"result\":\"PASSED\"}" +
",{\"uniformPath\":\"test3\",\"durationMillis\":123,\"result\":\"PASSED\"}]");
}

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
import retrofit2.Call;
import retrofit2.Response;
import retrofit2.Retrofit;
import retrofit2.converter.moshi.MoshiConverterFactory;
import retrofit2.converter.jackson.JacksonConverterFactory;
import retrofit2.http.POST;
import retrofit2.http.Query;

Expand Down Expand Up @@ -99,7 +99,7 @@ public void testAccessViaTiaClientAndReportUploadToTeamscale() throws Exception

testRun.endTestRun(true);
verify(client).uploadReport(eq(EReportFormat.TESTWISE_COVERAGE),
matches("\\Q{\"partial\":true,\"tests\":[{\"content\":\"content\",\"paths\":[],\"sourcePath\":\"test1\",\"uniformPath\":\"test1\"},{\"content\":\"content\",\"duration\":\\E[^,]*\\Q,\"message\":\"message\",\"paths\":[{\"files\":[{\"coveredLines\":\"1-4\",\"fileName\":\"Main.java\"}],\"path\":\"src/main/java\"}],\"result\":\"PASSED\",\"sourcePath\":\"test2\",\"uniformPath\":\"test2\"}]}\\E"),
matches("\\Q{\"partial\":true,\"tests\":[{\"uniformPath\":\"test1\",\"sourcePath\":\"test1\",\"content\":\"content\",\"paths\":[]},{\"uniformPath\":\"test2\",\"sourcePath\":\"test2\",\"content\":\"content\",\"duration\":\\E[^,]*\\Q,\"result\":\"PASSED\",\"message\":\"message\",\"paths\":[{\"path\":\"src/main/java\",\"files\":[{\"fileName\":\"Main.java\",\"coveredLines\":\"1-4\"}]}]}]}\\E"),
any(), any(), any(), any());
}

Expand Down Expand Up @@ -142,7 +142,7 @@ public void shouldHandleMissingRequestBodyForTestrunStartGracefully() throws Exc
new TestwiseCoverageAgent(mockOptions(port), null, reportGenerator);

ITestwiseCoverageAgentApiWithoutBody api = new Retrofit.Builder()
.addConverterFactory(MoshiConverterFactory.create())
.addConverterFactory(JacksonConverterFactory.create())
.baseUrl("http://localhost:" + port)
.build().create(ITestwiseCoverageAgentApiWithoutBody.class);
Response<List<PrioritizableTestCluster>> response = api.testRunStarted(false, null).execute();
Expand Down
2 changes: 1 addition & 1 deletion common-system-test/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,6 @@ dependencies {
api(project(":tia-client"))

api(libs.spark)
api(libs.moshi)
api(libs.jackson.databind)
api(libs.teamscaleLibCommons)
}
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
package com.teamscale.test.commons;

import com.squareup.moshi.JsonAdapter;
import com.squareup.moshi.Moshi;
import com.squareup.moshi.Types;
import com.teamscale.client.ClusteredTestDetails;
import com.teamscale.client.PrioritizableTest;
import com.teamscale.client.PrioritizableTestCluster;
import com.teamscale.client.TestWithClusterId;
import com.teamscale.report.testwise.model.TestwiseCoverageReport;
import com.teamscale.report.util.JsonUtils;
import spark.Request;
import spark.Response;
import spark.Service;
Expand Down Expand Up @@ -34,22 +32,13 @@
*/
public class TeamscaleMockServer {

private final JsonAdapter<List<PrioritizableTestCluster>> testClusterJsonAdapter = new Moshi.Builder().build()
.adapter(Types.newParameterizedType(List.class, PrioritizableTestCluster.class));

private final JsonAdapter<List<ClusteredTestDetails>> testDetailsJsonAdapter = new Moshi.Builder().build()
.adapter(Types.newParameterizedType(List.class, ClusteredTestDetails.class));

private final JsonAdapter<TestwiseCoverageReport> testwiseCoverageReportJsonAdapter = new Moshi.Builder().build()
.adapter(TestwiseCoverageReport.class);

/** All reports uploaded to this Teamscale instance. */
public final List<ExternalReport> uploadedReports = new ArrayList<>();
/** All user agents that were present in the received requests. */
public final Set<String> collectedUserAgents = new HashSet<>();

/** All tests that the test engine has signaled to Teamscale as being available for execution. */
public final Set<ClusteredTestDetails> availableTests = new HashSet<>();
public final Set<TestWithClusterId> availableTests = new HashSet<>();
private final Path tempDir = Files.createTempDirectory("TeamscaleMockServer");
private final Service service;
private List<String> impactedTests;
Expand Down Expand Up @@ -98,14 +87,14 @@ public TeamscaleMockServer disallowingStateChanges() {
* @throws IOException when parsing the report fails.
*/
public TestwiseCoverageReport parseUploadedTestwiseCoverageReport(int index) throws IOException {
return testwiseCoverageReportJsonAdapter.fromJson(uploadedReports.get(index).getReportString());
return JsonUtils.deserialize(uploadedReports.get(index).getReportString(), TestwiseCoverageReport.class);
}

private String handleImpactedTests(Request request, Response response) throws IOException {
collectedUserAgents.add(request.headers("User-Agent"));
availableTests.addAll(testDetailsJsonAdapter.fromJson(request.body()));
availableTests.addAll(JsonUtils.deserializeList(request.body(), TestWithClusterId.class));
List<PrioritizableTest> tests = impactedTests.stream().map(PrioritizableTest::new).collect(toList());
return testClusterJsonAdapter.toJson(Collections.singletonList(new PrioritizableTestCluster("cluster", tests)));
return JsonUtils.serialize(Collections.singletonList(new PrioritizableTestCluster("cluster", tests)));
}

private String handleReport(Request request, Response response) throws IOException, ServletException {
Expand Down
10 changes: 8 additions & 2 deletions gradle/libs.versions.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
[versions]
jetty = "9.4.53.v20231009"
jersey = "2.41"
jackson = "2.16.0"
# When upgrading JaCoCo to a newer version make sure to
# check the comment in the OpenAnalyzer.java, JaCoCoPreMain.java and CachingInstructionsBuilder.java
# and update the internal_xxxxxx hash included in the imports in LenientCoverageTransformer.java.
Expand All @@ -23,6 +24,12 @@ jersey-containerServletCore = { module = "org.glassfish.jersey.containers:jersey
jersey-mediaJsonJackson = { module = "org.glassfish.jersey.media:jersey-media-json-jackson", version.ref = "jersey" }
jersey-hk2 = { module = "org.glassfish.jersey.inject:jersey-hk2", version.ref = "jersey" }

jackson-bom = { module = "com.fasterxml.jackson:jackson-bom", version.ref = "jackson" }
jackson-annotations = { module = "com.fasterxml.jackson.core:jackson-annotations", version.ref = "jackson" }
jackson-databind = { module = "com.fasterxml.jackson.core:jackson-databind", version.ref = "jackson" }
jackson-jakarta-xmlbindAnnotations = { module = "com.fasterxml.jackson.module:jackson-module-jakarta-xmlbind-annotations", version.ref = "jackson" }
jackson-jakarta-rsJsonProvider = { module = "com.fasterxml.jackson.jakarta.rs:jackson-jakarta-rs-json-provider", version.ref = "jackson" }

jakarta-activation-api = { module = "jakarta.activation:jakarta.activation-api", version = "1.2.2" }

jacoco-core = { module = "org.jacoco:org.jacoco.core", version.ref = "jacoco" }
Expand All @@ -33,13 +40,12 @@ logback-core = { module = "ch.qos.logback:logback-core", version.ref = "logback"
logback-classic = { module = "ch.qos.logback:logback-classic", version.ref = "logback" }

retrofit-core = { module = "com.squareup.retrofit2:retrofit", version.ref = "retrofit" }
retrofit-converter-moshi = { module = "com.squareup.retrofit2:converter-moshi", version.ref = "retrofit" }
retrofit-converter-jackson = { module = "com.squareup.retrofit2:converter-jackson", version.ref = "retrofit" }

okhttp-core = { module = "com.squareup.okhttp3:okhttp", version.ref = "okhttp" }
okhttp-mockwebserver = { module = "com.squareup.okhttp3:mockwebserver", version.ref = "okhttp" }

spark = { module = "com.sparkjava:spark-core", version = "2.9.4" }
moshi = { module = "com.squareup.moshi:moshi", version = "1.15.0" }
jcommander = { module = "com.beust:jcommander", version = "1.82" }
teamscaleLibCommons = { module = "com.teamscale:teamscale-lib-commons", version = "9.3.0" }
commonsCodec = { module = "commons-codec:commons-codec", version = "1.16.0" }
Expand Down
3 changes: 1 addition & 2 deletions report-generator/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,7 @@ dependencies {
}
}

// Need to stay at 1.12.0 because 1.13.0 upwards uses multi release jars which breaks the gradle plugin tests
implementation("com.squareup.moshi:moshi:1.12.0")
implementation(libs.jackson.databind)

testImplementation(libs.jsonassert)
testImplementation(libs.teamscaleLibCommons)
Expand Down
Loading

0 comments on commit 790df3f

Please sign in to comment.