spring-projects / spring-data-geode

Spring Data support for Apache Geode
Apache License 2.0
52 stars 39 forks source link

Avoid test cases that test Spring Data Commons' internals [DATAGEODE-291] #336

Closed spring-projects-issues closed 4 years ago

spring-projects-issues commented 4 years ago

Oliver Drotbohm opened DATAGEODE-291 and commented

Some methods in MappingPdxSerializerUnitTests actually test the behavior of classes in Spring Data Commons. Especially around EntityInstantiators API. Those tests now break as they make (now changed) assumptions about the way that the Commons APIs behave. Let's remove those tests for now


Referenced from: commits https://github.com/spring-projects/spring-data-geode/commit/3e45bf63a3a567a8ccf9c4d7cd4c088c1f7b27e9, https://github.com/spring-projects/spring-data-geode/commit/4139d17009c2c84d17636f4cd858380f7701d845, https://github.com/spring-projects/spring-data-geode/commit/a395950033cf0986f9c0df97f481cbbfcf37a57c, https://github.com/spring-projects/spring-data-geode/commit/11058afd8f4ea8fe63b1bef11aa2d78b987a38b3

spring-projects-issues commented 4 years ago

John Blum commented

I am not a fan of removing tests!  Especially when you consider the tests involved and the importance of the MappingPdxSerializer doing the right thing!

 

As such, I have:

 

1) Reintroduced (reinstated) these [tests|https://github.com/spring-projects/spring-data-geode/blob/master/spring-data-geode/src/test/java/org/springframework/data/gemfire/mapping/MappingPdxSerializerIntegrationTests.java#L168-L214], this time in the MappingPdxSerializerIntegrationTests class as proper "Integration Tests" given the nature of these tests.

 

2) I also introduced proper [Unit Tests|https://github.com/spring-projects/spring-data-geode/blob/master/spring-data-geode/src/test/java/org/springframework/data/gemfire/mapping/MappingPdxSerializerUnitTests.java#L469-L508] to cover the functional/behavioral contract of the MappingPdxSerializer class w.r.t. its dependency on the SD Commons API and classes in question.