Skip to content

Conversation

4everTheOne
Copy link
Contributor

First of all, sorry for the long PR but this includes a lot of tests.

I'll try to explain what is going on in each commit to make it easier to understand.

Commit Structure

1. Implement the base tests for each ResolvedDeclaration;

The base tests are interfaces that can be reused to validate common behavior .

2. Create tests for javaparsermodel declarations and javassistmodel;

This tests combine the previous tests with additional ones to validate the behavior of the class.

3. Implement AssociableToAST on in classes that missed it.

With this tests some classes did not implement the AssociableToAST as expected. The classes that required change were:

com/github/javaparser/symbolsolver/javaparsermodel/declarations/
  ├── JavaParserEnumDeclaration.java
  ├── JavaParserFieldDeclaration.java
  ├── JavaParserParameterDeclaration.java
  ├── JavaParserPatternDeclaration.java
  ├── JavaParserTypeVariableDeclaration.java
  └── JavaParserVariableDeclaration.java

4. Improved code to match the expected behavior.

The issue was discussed at #3057 for different files, but same issue was seen here and fixed accordingly.
By default, some of the methods in ResolvedDeclaration have a default return value. An examples of this is the method ResolvedDeclaration#isField(). Some old classes instead of using the default behavior (returning false in the previous example) they throw an UnsupportedException.

5. JavaParserParameterDeclaration cleanup

Removed unnecessary method definitions since they are already defined in the super class. This helps by improve the line coverage and keep the same behavior.

6. JavaParserPatternDeclaration cleanup

The same as last one but for a different file.

@codecov
Copy link

codecov bot commented Feb 3, 2021

Codecov Report

Merging #3062 (58c35c1) into master (63fd4ec) will decrease coverage by 0.002%.
The diff coverage is 0.000%.

Impacted file tree graph

@@              Coverage Diff              @@
##            master     #3062       +/-   ##
=============================================
- Coverage   50.468%   50.466%   -0.003%     
=============================================
  Files          450       450               
  Lines        25105     25106        +1     
  Branches      3794      3794               
=============================================
  Hits         12670     12670               
- Misses       11456     11457        +1     
  Partials       979       979               
Flag Coverage Δ
AlsoSlowTests 50.466% <0.000%> (-0.003%) ⬇️
javaparser-core 50.466% <0.000%> (-0.003%) ⬇️
jdk-10 50.466% <0.000%> (-0.003%) ⬇️
jdk-11 50.466% <0.000%> (-0.003%) ⬇️
jdk-12 50.466% <0.000%> (-0.003%) ⬇️
jdk-13 50.466% <0.000%> (-0.003%) ⬇️
jdk-14 50.466% <0.000%> (-0.003%) ⬇️
jdk-15 50.466% <0.000%> (-0.003%) ⬇️
jdk-8 50.464% <0.000%> (-0.003%) ⬇️
jdk-9 50.466% <0.000%> (-0.003%) ⬇️
macos-latest 50.454% <0.000%> (-0.003%) ⬇️
ubuntu-latest 50.454% <0.000%> (-0.003%) ⬇️
windows-latest 50.454% <0.000%> (-0.003%) ⬇️

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

Impacted Files Coverage Δ
...ution/declarations/ResolvedPatternDeclaration.java 0.000% <0.000%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 63fd4ec...58c35c1. Read the comment docs.

@4everTheOne
Copy link
Contributor Author

Note that all tests have passed. The red cross is there due to lack of coverage of previous tests.

@jlerbsc
Copy link
Collaborator

jlerbsc commented Feb 4, 2021

There are a lot of changes in this making it difficult and time consuming to validate your work. Thanks for this work.

@jlerbsc
Copy link
Collaborator

jlerbsc commented Feb 4, 2021

It seems to be correct and the failing check is related only to codecov so i think there is no problem to merge this PR. Thank you for this contribution.

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