spring-boot icon indicating copy to clipboard operation
spring-boot copied to clipboard

Add simple API to JacksonTester for direct instantiation

Open ttddyy opened this issue 3 years ago • 4 comments

@JsonTest and json testers are hidden gems for tests with json.

Background

I have seen many test classes that create their own JsonUtils to load json files. The JSON testers in org.springframework.boot.test.json could do it and they are more powerful.

However, the creation of the json testers, for example, JacksonTester is somewhat geared toward the usage with @JsonTest.

When I want to use it outside of @JsonTest, instantiation requires reflection based instance injection or a bit verbose constructor APIs.

JacksonTester<Foo> tester;

@BeforeEach
void beforeEach(){
    // reflection based injection
    JacksonTester.initFields(this, objectMapper);
}
@Test
void myTest() {
  // it is not intuitive constructor to use
  JacksonTester<Foo> tester =  new JacksonTester<>(getClass(), ResolvableType.forClass(MySample.class), objectMappeer);
}

Proposal

To make direct instantiation simpler, it would be nice to add simple constructors or a builder.

For example, JacksonTester could have constructors that doesn't require resourceLoadClass argument:

// simple usage
JacksonTester(Class<T> type, ObjectMapper objectMapper)

// for fine tuning
JacksonTester(ResolvableType type, ObjectMapper objectMapper)

// for generics (above ResolvableType kind of covers it though)
JacksonTester(ParameterizedTypeReference<?> typeReference, ObjectMapper objectMapper)

Or with a builder:

// for simple class
JsonTester<MySample> tester = builder.type(MySamle.class).build();

// for collection
JsonTester<List<MySample>> tester = builder.forList(MySamle.class).build();

// for class with generics
JsonTester<Foo<Bar>> tester = builder.type(Foo.class, Bar.class).build();

This way, I can simply create an instance in a test method like:

@Test
void myTest() {
  JacksonTester<Foo> tester = new JacksonTester<>(Foo.class, objectMapper);
}

Implementation Details

The current public constructor takes:

  • Class<?> resourceLoadClass
  • ResolvableType type
  • ObjectMapper objectMapper

Within AbstractJsonMarshalTester, resourceLoadClass is enforced to be a non-null. However, it is essentially passed down to the ClassPathResource to get a classloader from the resourceLoadClass. In ClassPathResource, it is actually a nullable parameter. When the parameter is null, it falls back to the system class loader. I think in many cases it is fine, especially in test. Or, it could use the target type class to retrieve a classloader.

Passing ResolvableType as a constructor parameter is a bit unintuitive for direct instantiation. It could just take the target type class and internally convert it to a ResolvableType instance.

ttddyy avatar Jun 14 '21 22:06 ttddyy

The resourceLoadClass is optional but quite useful when you organize resources in the same package as your test. It allows you to do assertThat(tester.write(object)).isEqualToJson("expected.json") rather than assertThat(tester.write(object)).isEqualToJson("org/example/app/test/json/expected.json")

philwebb avatar Jun 15 '21 19:06 philwebb

I see for the usage. But resourceLoadClass is not optional if I directly instantiate JacksonTester.

new JacksonTester<>(null, ResolvableType.forClass(MySample.class), mapper);

The constructor of AbstractJsonMarshalTester performs non-null check on resourceLoadClass.

ttddyy avatar Jun 15 '21 20:06 ttddyy

I've started looking at this. A couple of thoughts have come from that:

  1. If we do this for JacksonTester, we should do it for GsonTester and JsonbTester as well
  2. While public, the API doesn't feel well-suited to encouraging more direct usage, particularly its generics

The second of these thoughts has me wondering if we should make this change at all. JacksonTester<Foo> tester = new JacksonTester<>(Foo.class, objectMapper); is nice but it becoming JacksonTester<Object> as soon as ResolvableType is involved is not.

The background above describes the motivation for the proposed changes:

I have seen many test classes that create their own JsonUtils to load json files. The JSON testers in org.springframework.boot.test.json could do it and they are more powerful.

If the goal is an API for loading JSON files, I'm not sure that JacksonTester (and its siblings) are necessarily the right answer. There's nothing Jackson- Gson- or Jsonb-specific about loading JSON after all. We've had some ideas in this area in the past but didn't see sufficient need for them. I wonder if this area is worth revisiting with a broader focus that doesn't just look at …Tester or MockRestServiceServer responses.

wilkinsona avatar Aug 10 '21 10:08 wilkinsona

I think there are two usecases for loading a json from resources.

One is for simple deserialization. Load an object from json file and pass it as an input to the test target method. I think this usecase doesn't need to tie to the tester classes.

Another usecase is for verification. Invoke the test target operation and verify the result(object or json string) against a json file. In this case, jsontester's assertion api(JsonContentAssert) provides a power.

ttddyy avatar Aug 12 '21 04:08 ttddyy