Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow @DataNeo4jTest to autoconfigure an embedded test instance #27151

Open
xenoterracide opened this issue Jul 5, 2021 · 6 comments
Open

Allow @DataNeo4jTest to autoconfigure an embedded test instance #27151

xenoterracide opened this issue Jul 5, 2021 · 6 comments
Labels
theme: testing Issues related to testing type: enhancement A general enhancement

Comments

@xenoterracide
Copy link
Contributor

xenoterracide commented Jul 5, 2021

spring-projects/spring-data-neo4j#2316 (comment)

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.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jul 5, 2021
@xenoterracide xenoterracide changed the title Allow @ Allow @DataNeo4jTest to autoconfigure an embedded test instance Jul 5, 2021
@wilkinsona
Copy link
Member

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 wilkinsona added for: team-meeting An issue we'd like to discuss as a team to make progress theme: testing Issues related to testing type: enhancement A general enhancement labels Jul 6, 2021
@wilkinsona
Copy link
Member

wilkinsona commented Jul 15, 2021

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

@xenoterracide
Copy link
Contributor Author

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
Copy link
Member

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
Copy link
Contributor Author

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
Copy link
Contributor Author

@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);
  }
}

@wilkinsona wilkinsona removed the for: team-meeting An issue we'd like to discuss as a team to make progress label Jul 26, 2021
@snicoll snicoll added this to the General Backlog milestone Jan 30, 2022
@snicoll snicoll removed the status: waiting-for-triage An issue we've not yet triaged label Jan 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme: testing Issues related to testing type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

4 participants