Skip to content

[ZEPPELIN-6221] Optimize Neo4jCypherInterpreterTest performance by reusing interpreter instance #4983

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

Closed
wants to merge 4 commits into from

Conversation

jongheon1
Copy link

@jongheon1 jongheon1 commented Jul 17, 2025

What is this PR for?

This PR optimizes the performance of Neo4jCypherInterpreterTest by eliminating redundant interpreter initialization.
Currently, each test method creates and closes a new Neo4jCypherInterpreter instance, which introduces unnecessary overhead when running against a Neo4j Testcontainer.
By using @testinstance(TestInstance.Lifecycle.PER_CLASS) and moving setup to @BeforeAll, we reduce initialization from 11 times to 1 time, improving test execution time.

What type of PR is it?

Improvement

Todos

  • - Use @testinstance(TestInstance.Lifecycle.PER_CLASS) annotation
  • - Move interpreter setup from @beforeeach to @BeforeAll
  • - Make interpreter and context static fields
  • - Verify all tests still pass

What is the Jira issue?

How should this be tested?

  1. Run the Neo4jCypherInterpreterTest suite: mvn test -Dtest=Neo4jCypherInterpreterTest
  2. Verify all 11 tests pass successfully
  3. Confirm test execution time is reduced compared to the previous implementation

Screenshots (if appropriate)

N/A

Questions:

  • Does the licenses files need update? No.
  • Is there breaking changes for older versions? No.
  • Does this needs documentation? No.

@Reamer
Copy link
Contributor

Reamer commented Jul 21, 2025

I have noticed a difference.
Before:
grafik
After:
grafik

In my eyes, 2 seconds does not justify changing the long-established standard from @TestInstance(TestInstance.Lifecycle.PER_METHOD) to @TestInstance(TestInstance.Lifecycle.PER_CLASS), therefore I reject the change. If another maintainer sees this differently, they are welcome to open this PR again.

@Reamer Reamer closed this Jul 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants