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

Add tests for NodeStack and rework #6009

Merged
merged 10 commits into from
Oct 27, 2024

Conversation

lorduke22
Copy link
Contributor

  • I have read CONTRIBUTING.md.
  • This pull request is all my own work -- I have not plagiarized it.
  • All filenames are in PascalCase.
  • All functions and variable names follow Java naming conventions.
  • All new algorithms have a URL in their comments that points to Wikipedia or other similar explanations.
  • All new code is formatted with clang-format -i --style=file path/to/your/file.java

@codecov-commenter
Copy link

codecov-commenter commented Oct 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 72.36%. Comparing base (a163816) to head (f2fef25).

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #6009      +/-   ##
============================================
- Coverage     72.37%   72.36%   -0.01%     
+ Complexity     4981     4979       -2     
============================================
  Files           652      652              
  Lines         17569    17564       -5     
  Branches       3385     3384       -1     
============================================
- Hits          12716    12711       -5     
  Misses         4373     4373              
  Partials        480      480              

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

@lorduke22 lorduke22 requested a review from siriak October 25, 2024 20:29
Copy link
Member

@siriak siriak left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@siriak siriak enabled auto-merge (squash) October 26, 2024 06:12
Copy link
Member

@siriak siriak left a comment

Choose a reason for hiding this comment

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

Please resolve conflicts

auto-merge was automatically disabled October 26, 2024 19:25

Head branch was pushed to by a user without write access

@lorduke22 lorduke22 requested a review from siriak October 26, 2024 19:29
Copy link
Member

@siriak siriak left a comment

Choose a reason for hiding this comment

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

Everything except print looks good

Copy link
Member

@siriak siriak left a comment

Choose a reason for hiding this comment

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

Please fix PR check

@lorduke22
Copy link
Contributor Author

I cannot solve this PR because during a CI/CD pipeline run with Infer, a potential Null Dereference was detected in the BinaryTreeTest class.
The analysis flagged the following line as a potential issue:

src/test/java/com/thealgorithms/datastructures/trees/BinaryTreeTest.java:38: error: Null Dereference(NULLPTR_DEREFERENCE)
BinaryTree$Node BinaryTree.getRoot() could be null (from the call to BinaryTree() on line 28) and is dereferenced.

To solve the build problem, if I wish, I can apply a temporary fix to prevent the test from failing on the BinaryTreeTest class (e.g., by adding a null check in getRoot())

@siriak Let me know how to proceed

@lorduke22 lorduke22 requested a review from siriak October 27, 2024 14:23
@siriak
Copy link
Member

siriak commented Oct 27, 2024

Hmm, that's odd. Could you try to fix the issue here or in a separate PR?

@Chiefpatwal
Copy link
Contributor

I cannot solve this PR because during a CI/CD pipeline run with Infer, a potential Null Dereference was detected in the BinaryTreeTest class. The analysis flagged the following line as a potential issue:

src/test/java/com/thealgorithms/datastructures/trees/BinaryTreeTest.java:38: error: Null Dereference(NULLPTR_DEREFERENCE) BinaryTree$Node BinaryTree.getRoot() could be null (from the call to BinaryTree() on line 28) and is dereferenced.

To solve the build problem, if I wish, I can apply a temporary fix to prevent the test from failing on the BinaryTreeTest class (e.g., by adding a null check in getRoot())

@siriak Let me know how to proceed

Hey @lorduke22, I’m facing the same conflict issue due to the failing Infer run check. I’m stuck and could use some guidance if you have any insights on resolving it.

@lorduke22
Copy link
Contributor Author

@siriak I should have solved the problem in this PR.
Hi @Chiefpatwal, the problem lies in the BinaryTreeTest class I see the getRoot() method may return null if the root node has been removed and there are no more nodes in the tree. This is why Infer reports a potential NullPointerException when attempting to access the date field in t.getRoot().data.
To resolve the error, I added a null check on getRoot() in test2.

image

Feel free to add the following control to your code

Copy link
Member

@siriak siriak left a comment

Choose a reason for hiding this comment

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

Great, thanks for fixing this!

@siriak siriak merged commit a2e526b into TheAlgorithms:master Oct 27, 2024
6 checks passed
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.

4 participants