Skip to content

Commit

Permalink
Add a currently-failing test for Java records and @Ignore it. (#2203)
Browse files Browse the repository at this point in the history
* Add a currently-failing test for Java records and `@Ignore` it.

Also do the Maven gymastics required to ensure that this test only runs on Java
versions ≥17. (It would also work on Java 16, but 17 is all we have in the CI.)

Fix some compilation problems I saw when running locally, which for some reason
don't show up in the CI.

* Suppress some new lint options that trigger `-Werror`.
We may fix these later. (Every test will need an explicit constructor!)

* Select Java version with maven.compiler.release and maven.compiler.testRelease.

Use `assumeNotNull` rather than an if-statement.

* Specify <release>11</release> for javadoc.

* Restore the @see for AccessibleObject.
  • Loading branch information
eamonnmcmanus authored Sep 27, 2022
1 parent 2591ede commit 441406c
Show file tree
Hide file tree
Showing 5 changed files with 119 additions and 7 deletions.
29 changes: 29 additions & 0 deletions gson/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@
</license>
</licenses>

<properties>
<excludeTestCompilation>**/Java17*</excludeTestCompilation>
</properties>

<dependencies>
<dependency>
<groupId>junit</groupId>
Expand Down Expand Up @@ -60,6 +64,18 @@
</excludes>
</configuration>
</execution>
<execution>
<id>default-testCompile</id>
<phase>test-compile</phase>
<goals>
<goal>testCompile</goal>
</goals>
<configuration>
<testExcludes>
<exclude>${excludeTestCompilation}</exclude>
</testExcludes>
</configuration>
</execution>
</executions>
</plugin>
<plugin>
Expand Down Expand Up @@ -222,4 +238,17 @@
</plugin>
</plugins>
</build>
<profiles>
<profile>
<id>JDK17</id>
<activation>
<jdk>[17,)</jdk>
</activation>
<properties>
<maven.compiler.testRelease>17</maven.compiler.testRelease>
<excludeTestCompilation></excludeTestCompilation>
<extraLintSuppressions>,-exports,-missing-explicit-ctor,-removal</extraLintSuppressions>
</properties>
</profile>
</profiles>
</project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
/*
* Copyright (C) 2022 Google Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.google.gson.functional;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThrows;

import com.google.gson.Gson;
import com.google.gson.annotations.SerializedName;
import java.util.Objects;
import org.junit.Ignore;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;

@RunWith(JUnit4.class)
@Ignore
public final class Java17RecordTest {
private final Gson gson = new Gson();

@Test
public void testFirstNameIsChosenForSerialization() {
MyRecord target = new MyRecord("v1", "v2");
// Ensure name1 occurs exactly once, and name2 and name3 don't appear
assertEquals("{\"name\":\"v1\",\"name1\":\"v2\"}", gson.toJson(target));
}

@Test
public void testMultipleNamesDeserializedCorrectly() {
assertEquals("v1", gson.fromJson("{'name':'v1'}", MyRecord.class).a);

// Both name1 and name2 gets deserialized to b
assertEquals("v11", gson.fromJson("{'name1':'v11'}", MyRecord.class).b);
assertEquals("v2", gson.fromJson("{'name2':'v2'}", MyRecord.class).b);
assertEquals("v3", gson.fromJson("{'name3':'v3'}", MyRecord.class).b);
}

@Test
public void testMultipleNamesInTheSameString() {
// The last value takes precedence
assertEquals("v3", gson.fromJson("{'name1':'v1','name2':'v2','name3':'v3'}", MyRecord.class).b);
}

@Test
public void testConstructorRuns() {
assertThrows(NullPointerException.class,
() -> gson.fromJson("{'name1': null, 'name2': null", MyRecord.class));
}

private static record MyRecord(
@SerializedName("name") String a,
@SerializedName(value = "name1", alternate = {"name2", "name3"}) String b) {
MyRecord {
Objects.requireNonNull(a);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.fail;
import static org.junit.Assume.assumeNotNull;

import com.google.gson.Gson;
import com.google.gson.GsonBuilder;
Expand All @@ -17,10 +18,10 @@
import com.google.gson.internal.ConstructorConstructor;
import com.google.gson.stream.JsonReader;
import com.google.gson.stream.JsonWriter;
import java.awt.Point;
import java.io.File;
import java.io.IOException;
import java.io.Reader;
import java.lang.reflect.Constructor;
import java.lang.reflect.Type;
import java.util.LinkedList;
import java.util.List;
Expand All @@ -40,7 +41,7 @@ public void close() throws IOException {
}

@Test
public void testBlockInaccessibleJava() {
public void testBlockInaccessibleJava() throws ReflectiveOperationException {
Gson gson = new GsonBuilder()
.addReflectionAccessFilter(ReflectionAccessFilter.BLOCK_INACCESSIBLE_JAVA)
.create();
Expand All @@ -58,8 +59,18 @@ public void testBlockInaccessibleJava() {
);
}

// But serialization should succeed for classes with only public fields
String json = gson.toJson(new Point(1, 2));

// But serialization should succeed for classes with only public fields.
// Not many JDK classes have mutable public fields, thank goodness, but java.awt.Point does.
Class<?> pointClass = null;
try {
pointClass = Class.forName("java.awt.Point");
} catch (ClassNotFoundException e) {
}
assumeNotNull(pointClass);
Constructor<?> pointConstructor = pointClass.getConstructor(int.class, int.class);
Object point = pointConstructor.newInstance(1, 2);
String json = gson.toJson(point);
assertEquals("{\"x\":1,\"y\":2}", json);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ private static Class<?> loadClassWithDifferentClassLoader(Class<?> c) throws Exc
}

@Test
@SuppressWarnings("removal") // java.lang.SecurityManager
public void testRestrictiveSecurityManager() throws Exception {
// Must use separate class loader, otherwise permission is not checked, see Class.getDeclaredFields()
Class<?> clazz = loadClassWithDifferentClassLoader(ClassWithPrivateMembers.class);
Expand Down
7 changes: 4 additions & 3 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@

<properties>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<javaVersion>7</javaVersion>
<maven.compiler.release>7</maven.compiler.release>
<extraLintSuppressions></extraLintSuppressions>
</properties>

<scm>
Expand Down Expand Up @@ -70,14 +71,13 @@
<artifactId>maven-compiler-plugin</artifactId>
<version>3.10.1</version>
<configuration>
<release>${javaVersion}</release>
<showWarnings>true</showWarnings>
<showDeprecation>true</showDeprecation>
<failOnWarning>true</failOnWarning>
<compilerArgs>
<!-- Enable all warnings, except for ones which cause issues when building with newer JDKs, see also
https://docs.oracle.com/en/java/javase/11/tools/javac.html -->
<compilerArg>-Xlint:all,-options</compilerArg>
<compilerArg>-Xlint:all,-options${extraLintSuppressions}</compilerArg>
</compilerArgs>
<jdkToolchain>
<version>[11,)</version>
Expand All @@ -92,6 +92,7 @@
<jdkToolchain>
<version>[11,)</version>
</jdkToolchain>
<release>11</release>
<!-- Exclude `missing` group because some tags have been omitted when they are redundant -->
<doclint>all,-missing</doclint>
<!-- Link against newer Java API Javadoc because most users likely
Expand Down

0 comments on commit 441406c

Please sign in to comment.