Repository navigation
fix java neo4j tests - #4503
fix java neo4j tests#4503
Conversation
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the integration tests in ConstraintsIndicesIT.java. Specifically, it updates the Neo4j5CommunityIT and Neo4j44CommunityIT nested classes to use instance-level @before and @after lifecycle annotations instead of class-level @BeforeClass and @afterclass annotations, ensuring the neo4jClient is initialized and cleaned up per test. It also fixes a copy-paste bug in Neo4j44EnterpriseIT where the wrong class name was passed to setupClient. There are no review comments, so I have no feedback to provide.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4503 +/- ##
============================================
- Coverage 56.87% 56.85% -0.02%
+ Complexity 8088 7626 -462
============================================
Files 1164 1164
Lines 74114 74114
Branches 8735 8735
============================================
- Hits 42150 42136 -14
- Misses 29164 29176 +12
- Partials 2800 2802 +2
🚀 New features to boost your workflow:
|
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses flaky integration tests in the Neo4j template suite caused by shared resource contention in Neo4j Community Edition. By moving from class-level to method-level resource management, each test now runs in an isolated container, preventing race conditions where concurrent tests modify the same shared database instance. Additionally, a minor configuration fix was applied to the Enterprise Edition test setup. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request refactors the integration tests in ConstraintsIndicesIT.java by converting static class-level setup and cleanup methods (@BeforeClass/@afterclass) to instance-level methods (@Before/@after) for community editions, and fixing a class name typo in Neo4j44EnterpriseIT. The feedback suggests prefixing the test identifier with the class's simple name in Neo4j5CommunityIT and Neo4j44CommunityIT to avoid resource naming collisions during parallel test execution.
see issue here https://github.com/GoogleCloudPlatform/DataflowTemplates/actions/runs/40684579964
Problem: In ConstraintsIndicesIT, Neo4j5CommunityIT and Neo4j44CommunityIT shared a single class-level Neo4jResourceManager instance (@BeforeClass) across test methods that run in parallel (parallel='classesAndMethods'). Unlike Neo4j Enterprise Edition, Neo4j Community Edition only supports a single "neo4j" database, so canResetDatabase() created an index (CREATE INDEX FOR (n:From) ON n.name) in the shared "neo4j" database while doesNotCreateExtraIndicesWhenImportingRelationships() and doesNotCreateExtraIndicesWhenImportingNodes() concurrently asserted that no non-lookup indexes existed (count = 0), causing flaky test failures whenever their Dataflow jobs finished before canResetDatabase() reset the database.
Fix: Scope Neo4jResourceManager in Neo4j5CommunityIT and Neo4j44CommunityIT to the test method level (@before / @after) so each parallel test method runs against its own isolated Neo4j Community container, while keeping class-level containers for the Enterprise Edition test classes that isolate tests via per-test databases.
Also, update naming of one class.