Skip to content

test(server-test): enable run single unit test#2940

Merged
imbajin merged 3 commits intoapache:masterfrom
JisoLya:single-UT
Jan 29, 2026
Merged

test(server-test): enable run single unit test#2940
imbajin merged 3 commits intoapache:masterfrom
JisoLya:single-UT

Conversation

@JisoLya
Copy link
Contributor

@JisoLya JisoLya commented Jan 20, 2026

Purpose of the PR

  • close #xxx

Main Changes

Verifying these changes

  • Trivial rework / code cleanup without any test coverage. (No Need)
  • Already covered by existing tests, such as (please modify tests here).
  • Need tests and can be verified as follows:
    • xxx

Does this PR potentially affect the following parts?

Documentation Status

  • Doc - TODO
  • Doc - Done
  • Doc - No Need

@dosubot dosubot bot added size:XS This PR changes 0-9 lines, ignoring generated files. tests Add or improve test cases labels Jan 20, 2026
@Tsukilc Tsukilc requested a review from Copilot January 26, 2026 08:53
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR enables running individual unit tests from the CoreTestSuite without executing the entire test suite. Previously, running a single test class directly would fail because the graph initialization only occurred when running the full CoreTestSuite with JUnit's Suite runner.

Changes:

  • Removed unused Assert import since Assert.assertNotNull(graph) assertion is no longer needed
  • Modified graph() method to perform lazy initialization when graph is null, ensuring tests can run independently

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@dosubot dosubot bot added size:S This PR changes 10-29 lines, ignoring generated files. and removed size:XS This PR changes 0-9 lines, ignoring generated files. labels Jan 27, 2026
synchronized (CoreTestSuite.class){
if (graph == null) {
initEnv();
init();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

‼️ Critical: Exception handling concern in lazy initialization

The initEnv() and init() methods can throw exceptions. If an exception occurs inside the synchronized block:

  1. graph remains null
  2. Subsequent calls will retry initialization (possibly desirable, but may cause repeated failures)

Consider whether failed initialization should be remembered to provide a clearer error message on subsequent calls, or at minimum document this behavior.

Also, there's an ordering dependency: initEnv() must complete before init(). While this works with the current registered flag, consider whether you need additional error handling if initEnv() succeeds but init() fails.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

init();
} catch (Throwable e) {
LOG.error("Failed to initialize HugeGraph instance", e);
graph = null;
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The assignment graph = null is redundant here. At this point in the code flow, graph is guaranteed to be null because it's only assigned a value in the init() method at line 95. This redundant assignment has no effect and can be removed.

Copilot uses AI. Check for mistakes.
@JisoLya JisoLya requested a review from imbajin January 29, 2026 07:44
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Jan 29, 2026
@imbajin imbajin merged commit 9babe49 into apache:master Jan 29, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm This PR has been approved by a maintainer size:S This PR changes 10-29 lines, ignoring generated files. tests Add or improve test cases

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants