spring-projects / spring-boot

Spring Boot
https://spring.io/projects/spring-boot
Apache License 2.0
74.56k stars 40.55k forks source link

Allow @DataNeo4jTest to autoconfigure an embedded test instance #27151

Open xenoterracide opened 3 years ago

xenoterracide commented 3 years ago

https://github.com/spring-projects/spring-data-neo4j/issues/2316#issuecomment-873918857

reading https://docs.spring.io/spring-data/neo4j/docs/current/reference/html/#dataneo4jtest

and wondering, couldn't this be easier? I mean how often am I going to care about the actual username and password, couldn't I just inject, for example, a Neo4j and have it configured by default when I use @DataNeo4jTest depending on what's on the classpath. I don't really see why the sample test should need to do anything other than this (well, it doesn't seem as this is setting up any data, but other than that...)

@DataNeo4jTest
class MovieRepositoryTest {

        @Test
        public void findSomethingShouldWork(@Autowired Neo4jClient client) {

                Optional<Long> result = client.query("MATCH (n) RETURN COUNT(n)")
                        .fetchAs(Long.class)
                        .one();
                assertThat(result).hasValue(0L);
        }
}

to contrast the examples in neo4j to reinforce my ask

@DataJpaTest
class MyRepositoryTests {

    @Autowired
    private TestEntityManager entityManager;

    @Autowired
    private UserRepository repository;

    @Test
    void testExample() throws Exception {
        this.entityManager.persist(new User("sboot", "1234"));
        User user = this.repository.findByUsername("sboot");
        assertThat(user.getUsername()).isEqualTo("sboot");
        assertThat(user.getEmployeeNumber()).isEqualTo("1234");
    }

}

the data jpa test doesn't require you to spend time in each test configuring the entity manager.

wilkinsona commented 3 years ago

There's some similarity here with what you currently have to do to use Testcontainers where you also use @DynamicPropertySource to set one or more properties, this time based on the container's settings. That's something that we'd like to improve. It's got a JDBC focus at the moment, but I wonder if there's something more general purpose that we can do that knows how to map from a Testcontainer, embedded Neo4j instance, etc to some properties or beans without the user having to do much.

wilkinsona commented 3 years ago

As part of this, we should consider adding dependency management for org.neo4j.test:neo4j-harness.

xenoterracide commented 3 years ago

what is wrong with this solution-ish (might need to be dynamically registered based on class/jar present?)

@DataNeo4jTest
@ExtendWith(AbstractMapToObjectConverterTest.Neo4jExtension.class)
class AbstractMapToObjectConverterTest {

  private final TestNodeRepository repository;

  @Autowired
  AbstractMapToObjectConverterTest(TestNodeRepository repository) {
    this.repository = repository;
  }

  @Test
  void decompose() {
  }

  @Test
  void compose() {
    var aString = "hello";
    var saved = repository.save(new TestNode(new TestComposite(aString)));
    assertThat(saved.getTestComposite().getaString()).isEqualTo(aString);
  }

  static class Neo4jExtension implements BeforeAllCallback, AfterAllCallback {

    private @Nullable Neo4j neo4j;

    @Override
    public void afterAll(ExtensionContext context) throws Exception {
      if ( neo4j != null ) {
        neo4j.close();
      }
    }

    @Override
    public void beforeAll(ExtensionContext context) throws Exception {
      neo4j = Neo4jBuilders.newInProcessBuilder()
        .withDisabledServer()
        .build();

      System.setProperty("spring.neo4j.uri", this.neo4j.boltURI().toString());
      System.setProperty("spring.neo4j.authentication.username", "neo4j");
    }
  }
}
wilkinsona commented 3 years ago

I think it's heading in the right direction, but we need something that isn't tied to JUnit 5 and that doesn't use system properties.

We might end up with something that's specific to Neo4j, but before we do that we want to explore the possibility of providing something more general that makes it easy to do this sort of thing whether you're using Testcontainers, Neo4j's test harness, or something else entirely.

xenoterracide commented 3 years ago

I'm investigating the dynamic properties now, as I'm not certain how to do that without an annotated method. As far as Junit 5, spring extension is using that? so what would be wrong with having another. I understand it might be more code now for you, but maybe it's a good stopgap?

xenoterracide commented 3 years ago

@philwebb it looks like I can't implement with DynamicPropertyRegistry without putting it on a test class? is that accurate? honestly this is kind of hard to implement (even if it's not generically not junit 5, which I don't actually see a problem with), maybe some of the API's there could change? as a lot of it is private or package protected. and not in the context. This is what I've got currently with source diving.

package com.xenoterracide.ppm.util.modelgraph;

import com.google.errorprone.annotations.Var;
import org.junit.jupiter.api.extension.AfterAllCallback;
import org.junit.jupiter.api.extension.BeforeAllCallback;
import org.junit.jupiter.api.extension.ExtensionContext;
import org.junit.jupiter.api.extension.ExtensionContext.Namespace;
import org.neo4j.harness.Neo4j;
import org.neo4j.harness.Neo4jBuilders;
import org.springframework.context.support.GenericApplicationContext;
import org.springframework.core.env.Environment;
import org.springframework.core.env.PropertySources;
import org.springframework.test.context.DynamicPropertyRegistry;
import org.springframework.test.context.TestContextManager;
import org.springframework.test.context.junit.jupiter.SpringExtension;

class Neo4jExtension implements BeforeAllCallback, AfterAllCallback {

  /** can't access {@link SpringExtension.TEST_CONTEXT_MANAGER_NAMESPACE } **/
  private static final Namespace SPRING_TEST_NS = Namespace.create(SpringExtension.class);

  private static GenericApplicationContext getApplicationContext( ExtensionContext ext) {
    return (GenericApplicationContext) ext.getRoot()
      .getStore(SPRING_TEST_NS)
      .get(ext.getRequiredTestClass(), TestContextManager.class )
      .getTestContext()
      .getApplicationContext();
  }

  @Override
  public void afterAll(ExtensionContext context) throws Exception {
    getApplicationContext(context).getBean(Neo4j.class).close();
  }

  @Override
  public void beforeAll(ExtensionContext context) throws Exception {
    var springContext = getApplicationContext(context);
    springContext.registerBean(Neo4j.class, () -> Neo4jBuilders.newInProcessBuilder()
       .withDisabledServer()
       .build());

    var neo4j = springContext.getBean(Neo4j.class);

    var ps = springContext.getEnvironment().getPropertySources()
      .get("Dynamic Test Properties");

    registry.add("spring.neo4j.uri", neo4j::boltURI);
    registry.add("spring.neo4j.authentication.username", () -> "neo4j");
    registry.add( "spring.neo4j.authentication.password", () -> null);
  }
}