Skip to content

Conversation

@sapienza88
Copy link
Contributor

Reason for this PR

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

@codecov-commenter
Copy link

codecov-commenter commented Sep 22, 2025

Codecov Report

❌ Patch coverage is 48.38710% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.51%. Comparing base (b7768ad) to head (03c7c42).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
...c/main/java/org/apache/graphar/info/GraphInfo.java 48.38% 14 Missing and 2 partials ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main     #773      +/-   ##
============================================
+ Coverage     76.48%   76.51%   +0.02%     
- Complexity      535      564      +29     
============================================
  Files            82       83       +1     
  Lines          8638     8720      +82     
  Branches       1001     1011      +10     
============================================
+ Hits           6607     6672      +65     
- Misses         1815     1828      +13     
- Partials        216      220       +4     
Flag Coverage Δ
java-info 81.79% <48.38%> (-0.21%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@sapienza88
Copy link
Contributor Author

sapienza88 commented Sep 22, 2025

@yangxk1 Do we need to consider the adjacency list when deleting a vertex/edge? where do you test addVertex() in GraphInfo.java ?

@yangxk1
Copy link
Contributor

yangxk1 commented Sep 23, 2025

Do we need to consider the adjacency list when deleting a vertex/edge?

Do you mean that delete adjacency list in edgeInfo?

where do you test addVertex() in GraphInfo.java ?

I don't think we tested addVertex, can you add tests for them in GraphInfoTest/EdgeInfoTest/VertexInfoTest by the way?

@sapienza88
Copy link
Contributor Author

Do we need to consider the adjacency list when deleting a vertex/edge?

Do you mean that delete adjacency list in edgeInfo?

where do you test addVertex() in GraphInfo.java ?

I don't think we tested addVertex, can you add tests for them in GraphInfoTest/EdgeInfoTest/VertexInfoTest by the way?

Ok, where is the yaml file, to load a sample testing graph from which i can add or remove vertex and validate?

@yangxk1
Copy link
Contributor

yangxk1 commented Sep 23, 2025

In GraphInfoTest::setup, a GraphInfo has been loaded and parsed, you can test method by it.

@yangxk1
Copy link
Contributor

yangxk1 commented Sep 26, 2025

@unical1988 , please note that runs git submodule update --remote use the latest test data

Copy link
Contributor

@yangxk1 yangxk1 left a comment

Choose a reason for hiding this comment

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

Good job!
please run git submodule update --remote to update testing data submodule

<version>${spotless-maven-plugin.version}</version>
<configuration>
<java>
<!--<includes>
Copy link
Contributor

Choose a reason for hiding this comment

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

please remember to delete this line

@yangxk1 yangxk1 changed the title add remove vertex edge, TODO: tests [WIP] add remove vertex edge, TODO: tests Oct 10, 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