INRIA / spoon

Spoon is a metaprogramming library to analyze and transform Java source code. :spoon: is made with :heart:, :beers: and :sparkles:. It parses source files to build a well-designed AST with powerful analysis and transformation API.
http://spoon.gforge.inria.fr/
Other
1.75k stars 349 forks source link

track progress of @Rohitesh-Kumar-Jain - GSoC 2021 student #3967

Closed Rohitesh-Kumar-Jain closed 3 years ago

Rohitesh-Kumar-Jain commented 3 years ago

Hi all,

This issue is for keeping track of my deliverables and work in the Google Summer of Code, also this issue can be used for communication between mentors/org members and the student.

Org members feel free to drop suggestions and feedback regarding the GSoC program : )

Timeline

Here's a link to the official GSoC timeline.

Phase Work Summary Related PRs
Community Bonding
(May 17 – June 7)
Migrated CtTypeReferenceTest, AstCheckerTest, FilterTest, ProcessingTest, CtGenerationTest, CorrectIdentifierTest to Junit 5 #3969, #3960,
#3959, #3958, #3956, #3946
Week 1
(June 7 – 13)
Improved tests for NameFilter & SpoonTagger class, migrated all the tests in template directory to Junit5, had a discussion about the location & extraction of tests #3986, #3982, #3975, #3978
Week 2
(June 14 – 20)
This week I transitioned from improving existing tests to writing new tests, and added tests for abstractTemplate and Substitution class #3990, #3995
Week 3
(June 21 – 27)
Added tests for insertAllNestedTypes, insertGeneratedNestedTypes, insertAllConstrutors, insertConsturctor, SpoonTreeBuilder.enter, SpoonTreeBuilder.exit, SpoonTreeBuilder.createNode. Migrated all spoon.refactoring, spoon.support and spoon.testing tests to Junit5 #3998, #4005, #4006, #4007, #4009
Week 4
(June 28 – July 4)
Added test for Substitution.insertAllSuperInterfaces, substituteMethodBody, substituteStatement, substituteFieldDefaultExpression #4012, #4016, #4018
Week 5
(July 5 – 11)
Added tests for asssertNotNull, assertExists, assertIsSame #4034
Week 6
(July 12 – 18)
added test for createPackageFile, createModuleFile, CtCommentImpl.equals, migrated few test classes to Junit5 #4043, #4045, #4053
Week 7
(July 19 – 25)
Added test for removeAnnotation , raised #4060 #4056
Week 8
(July 26 – August 1)
added test for CtCase.insertBegin, CtStatementList.insertBegin, fixed #4060 #4063, #4065
Week 9
(August 2 – 8)
added test for createReference, wrote test for CtCase and CtStatementList #4078, #4082, #4089
Week 10
(August 9 – 16)
added test for hasFieldConflict, hasNestedTypeConflict, removeArgument and removeActualTypeArgument #4088, #4091
Submission Period (August 16 - 23) Drafed Final Project report, yet to finalize and make submission Final Project Report
Organization's submission phase
(August 23 – 30)
Rohitesh-Kumar-Jain commented 3 years ago

Hi @nharrand, @monperrus and @slarse.

I wanted to ask what is the best approach to write tests for Spoon. (I wanted to ask what steps should I follow ?)

Currently, like for writing test for NameFilter class. My approach was to first try to read the class itself and tried to follow the Classes and Interfaces the class is extending or implementing but one class/interface leads to another one and after some time I realize I came far from the original class.

After that, I took a look at the test report generated by running Descartes, and realised that the line coverage can be improved. So wrote a test for that.

I would be glad if someone may suggest the steps to write a test, and what should I be focusing on while writing one.

slarse commented 3 years ago

After that, I took a look at the test report generated by running Descartes, and realised that the line coverage can be improved. So wrote a test for that.

Just to clarify, Descartes is not meant to measure line coverage. It does measure line coverage, but its purpose is to measure test strength through extreme mutation testing. It's the mutation testing that takes time, while standard code coverage metrics are very quick to compute.

To just get code coverage (line and branch), all you need to do is to run Spoon's test suite and then generate a coverage report with Jacoco,

mvn clean test jacoco:report

You can then find the coverage report as HTML in target/site/jacoco/index.html.

Similarly, you can also get coverage metrics inside of most modern IDEs, and each pull request will also have a status check from Coveralls stating the coverage. In IntelliJ IDEA, you can even run Jacoco inside of it, or just use IDEA's own coverage metrics (avoid sampling mode, it doesn't give branch coverage).

slarse commented 3 years ago

As I'm not sure how familiar you are with different test metrics, I figured I'd clarify the purpose of code coverage and the mutation testing that Descartes does:

With standard code coverage metrics (line and branch), we can find untested code. If code is not covered, it's definitely not being tested. The pro of this is that it's computationally very cheap to compute these metrics, but the con is that code being covered by tests does not imply that it's well tested. Here's an extreme example:

public void add(int lhs, int rhs) {
    return 22;
}

@Test
void testAdd() {
    add(2, 2);
}

The above test gives full coverage of the add method, but it doesn't test anything other than the method not crashing. So, the coverage is full, but the testing is very weak.

With Descartes, we can find untested code as well, but more importantly we can find poorly tested code. That is to say, code where we have coverage, but mutating the code does not cause any tests to fail (the mutants are not killed). You can read more about the mutations of Descartes in its docs, but I thought the docs didn't really provide that most fundamental motivation for why we want to do mutation testing.

Rohitesh-Kumar-Jain commented 3 years ago

With standard code coverage metrics (line and branch), we can find untested code. If code is not covered, it's definitely not being tested.

Thanks for clarifying the purpose of the code coverage and the mutation testing : )

So this means that perfect testing means 100% code coverage and that coverage should be tested by Descartes for checking their strength?

Rohitesh-Kumar-Jain commented 3 years ago

In IntelliJ IDEA, you can even run Jacoco inside of it, or just use IDEA's own coverage metrics (avoid sampling mode, it doesn't give branch coverage).

Thanks for sharing, I wasn't aware of this.

slarse commented 3 years ago

So this means that perfect testing means 100% code coverage and that coverage should be tested by Descartes for checking their strength?

Short answer: no.

Long answer: This is a very complex topic with a lot of different views. But right off the bat, I can tell you that there is no such thing as perfect testing. At best we can speak in relative terms: X is better than Y.

When it comes to code coverage, it's generally accepted that higher coverage is better than lower coverage. The problem that spawns from that is that developers write crappy tests just to satisfy coverage goals. Consider for example my test for testAdd: it satisfies to coverage goal of 100% coverage, but it's a useless test that brings a false sense of security. I'd say that good testing with lower coverage is better than bad testing with higher coverage any day of the weak.

I can't stress this enough, high code coverage does not imply good testing. The only certain thing you can read from line/branch coverage is what isn't tested at all.

Descartes is one step above code coverage. It does extreme mutation testing (do read the docs on what that means!), and as such it can only detect completely worthlessly tested code. Worthlessly tested code is however surprisingly common, perhaps due to code coverage culture being what it is.

Note: When I say code coverage, I mean the standard line and branch metrics. It should however be noted that there are stronger metrics, for example just one step up in strength we have path coverage, and there are stronger metrics that measure coverage based on data flow. But line and branch coverage are pretty much the only coverage metrics in wide use.

Rohitesh-Kumar-Jain commented 3 years ago

high code coverage does not imply good testing

Thanks, I have got the point, will keep this in mind : )

monperrus commented 3 years ago

high code coverage does not imply good testing

Fully agree. High coverage with strong assertions means good testing. Jacoco is responsible for checking the former, Descartes is responsible for checking the latter.

Rohitesh-Kumar-Jain commented 3 years ago

Hi,

Substitutuion.java, is big and a poorly tested class with mutation coverage of only 42% and test strength of 65%.

Screenshot 2021-06-14 at 6 47 07 PM

It's a big class and the test resources are also scattered

Screenshot 2021-06-14 at 6 52 06 PM

I am getting confused about where to start looking, plus it seems to me that this is some sort of utility class, so many methods are being indirectly tested?

@nharrand I would be glad if you may suggest something on how to get started on this file, how to find test resources, and where to write tests?

Rohitesh-Kumar-Jain commented 3 years ago

I was aiming to improve the strength of this overloaded function insertAllSuperInterfaces, but I am unable to its existing tests, how to find those tests?

Screenshot 2021-06-14 at 7 37 21 PM
Rohitesh-Kumar-Jain commented 3 years ago

@monperrus, @slarse & @nharrand

I have a question, we have a method called insertAllfields in a class called Substitution, that method isn't being used anywhere, plus the visibility of the substitution constructor is private, so how can I test that method?

Screenshot 2021-06-15 at 8 28 54 PM

slarse commented 3 years ago

@Rohitesh-Kumar-Jain The method is static. You don't need to, even should not, instantiate the class to use the method. Simply call the method directly on the class (i.e. Substitution.insertAllFields(someType, someTemplate)).

The constructor for Substitution is private as it is a utility class that isn't meant to be instantiated.

Rohitesh-Kumar-Jain commented 3 years ago

This is an untested class with low usage, 6/8 mutations are for getters and 1 is for initializer. Should I write tests for all getters and the initializer as well?

@slarse, @nharrand, @monperrus ?

Screen Shot 2021-06-29 at 15 08 13-fullpage

slarse commented 3 years ago

I don't know anything about this GUI package, but I don't think it's widely used. Unit testing each and every method seems like a waste of effort to me. Testing GUIs is also a traditionally very difficult task, and you typically want to employ some GUI testing framework for it.

@nharrand and @monperrus might have some more info here, but I think testing this particular package is of very low priority to the project.

Rohitesh-Kumar-Jain commented 3 years ago

@slarse, @monperrus & @nharrand,

Writing tests for classes such as MethodCallStack, is trivial I guess?

nharrand commented 3 years ago

@slarse, @monperrus & @nharrand,

Writing tests for classes such as MethodCallStack, is trivial I guess?

Hi @Rohitesh-Kumar-Jain , It is trivial to write test sthat directly call the classe itself, and hence not necessarily important. But thinking about what other class should be tested and indirectly call this one can be more interesting.

Rohitesh-Kumar-Jain commented 3 years ago

Hi @monperrus and @nharrand,

I would be glad if you may go through the bug #4060, and give suggestions on that.

I feel, that probably also lies within the scope of my work : )

Rohitesh-Kumar-Jain commented 3 years ago

@monperrus, @nharrand, and @slarse I am sharing the draft of the final project report. I have changed the setting to anyone with the link can comment so feel free to add your suggestions and request changes.

Here are the guidelines that I have kept in my mind.

monperrus commented 3 years ago

@Rohitesh-Kumar-Jain the final report LGTM. Very professional. We're lucky to have you as GSOC student :)

slarse commented 3 years ago

Agreed, it looks good, and congratulations on making such a large amount of meaningful contributions in such a short amount of time!

Rohitesh-Kumar-Jain commented 3 years ago

Thank you for your kind words 😄

I am lucky to be got selected with Castor !

I am glad that my contributions were meaningful.

If I ever plan to visit Sweden for a short holiday to see the northern lights, I will definitely come and meet you all : )

I will also check Swedish Schools if I will be applying for MS.

But in these Covid-times, it's hard to say when, due to international travel restrictions and students preferring home universities over risking going abroad.

algomaster99 commented 3 years ago

@Rohitesh-Kumar-Jain I haven't really tracked your work during the summers but the report and your PRs are very comprehensive so it was really easy for me to understand your project. Thanks for documenting your work and that too so well!

monperrus commented 3 years ago

GSOC has ended.

@Rohitesh-Kumar-Jain a big big thank you for your work on Spoon this summer. We wish you the very best for your career.

Rohitesh-Kumar-Jain commented 3 years ago

@Rohitesh-Kumar-Jain a big big thank you for your work on Spoon this summer. We wish you the very best for your career.

Thank you for your kind words.

Honestly I don't know what my career will be, I have switched flanks diagonally so far, I have reserved latter time of my studies towards exploring research and academics, and currently focusing on getting a strong command in problem solving and Data Strutures & Algorithms.