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

62 add test coverage for MethodAnalysis #111

Merged
merged 2 commits into from
Aug 25, 2022

Conversation

ade3p
Copy link
Contributor

@ade3p ade3p commented Aug 23, 2022

Issue #62: Add test coverage for FindMethodInvocationsAnalysis and FindMethodDeclarationsAnalysis

Hi, along with the tests I have made some changes to a couple of files (due to same kept the changes in a draft PR) - please find the reason below:

MatchedMethodResult:

  • line#9: updated the access specifier to public as its instance is required in test package for expectedResult comparison.
  • line#22: overloaded the constructor to accept the matchedNode as String - this change I didn't want to make but for creating expectedResult object it is required to create an instance of this class. Since its not possible to explicitly create an ASTNode and just for testing purposes - it doesn't seem like a good idea to parse the ExampleClass and visit/search for the node (basically it would be redundant as to what is being done for the FindMethodInvocationAnalysis itself).
    Also ASTNode.toString() i.e., string value for ASTNode is stored in the member matchedNode anyway.
  • line#40: added equals() implementation for comparison of the objects

MethodAnalysisResult:

  • line#13: added public access specifier for same reason as above.

DescribedPredicate:

  • line#33: implemetation override for equals()/hashcode()

With these two tests was able to bump the overall coverage to 80.1%
PFA the screen captures for individual class coverage.

astra3

astra1

astra2

@ade3p ade3p marked this pull request as ready for review August 24, 2022 10:29

@Test
public void testMethodDeclarationsAnalysis() {
FindMethodDeclarationsAnalysis analysis = new FindMethodDeclarationsAnalysis(getMethodMatcherSet());
Copy link
Member

Choose a reason for hiding this comment

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

Astra uses Java 11, so getMethodMatcherSet() could probably be replaced with Set.of(getMethodMatcher()).

@RadikalJin
Copy link
Member

Excellent stuff, thank you for your contribution to Astra @ade3p. Merging now.

@RadikalJin RadikalJin merged commit 9c66130 into alfasoftware:main Aug 25, 2022
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.

2 participants