search

VinodAnandan / sonar-pitest

43 stars 30 forks source link

pitest-plugin 1.0 for Sonar 5.0 #4

Closed gmuecke closed 9 years ago

gmuecke commented 9 years ago

The pull request contains:

alexvictoor commented 9 years ago

It looks like you spend quite some time on this PR, but sorry some changes are needed on my side in order to accept it. Please reformat the code to match coding rules (https://github.com/SonarSource/sonar-developer-toolset#code-style) Also we try to link every single change to a jira ticket. Try to link commits with jira tickets. Some commits are linked to issues (compatibility with sonarqube 5.0) and other to new features that are described nowhere. Please squash commits related to production code and tests together Please remove unnecessary comments in the code such as "logger: SLF4J Logger for this class"

gmuecke commented 9 years ago

Hi Alex,

I will have the issues fixed, what about the new features that have no corresponding jira-ticket? do we create new tickets for those?

On Mon, Mar 2, 2015 at 11:49 PM, Alexandre Victoor <notifications@github.com

wrote:

It looks like you spend quite some time on this PR, but sorry some changes are needed on my side in order to accept it. Please reformat the code to match coding rules ( https://github.com/SonarSource/sonar-developer-toolset#code-style) Also we try to link every single change to a jira ticket. Try to link commits with jira tickets. Some commits are linked to issues (compatibility with sonarqube 5.0) and other to new features that are described nowhere. Please remove unnecessary comments in the code such as "logger: SLF4J Logger for this class"

— Reply to this email directly or view it on GitHub https://github.com/SonarCommunity/sonar-pitest/pull/4#issuecomment-76844435 .

gmuecke commented 9 years ago

Hi Alex,

I'm about to rebase my commits to add the reference to the Jira tickets.

The ticket "SONARPLUGINS-2869 A separate violation type for every mutation.." is rather obvious and easy to assign. The issue "SONARPLUGINS-3818 Fix compatibility with SonarQube 5.0" is bit narrow focused to reflect all the changes and I could not pin down a single commit to this issue that has no other aspects changed. How about creating an "Improvement"-ticket for the pitest 1.0 plugin and resolve the issue 3818 (fixed/won't fix) as soon as the improvement is done? It's easier for me to assign all the non-functional changes (like package reorg etc) to such a more general improvement.

Unfortunately I'm not authorized to create tickets for the SonarQube Plugins project.

On Tue, Mar 3, 2015 at 5:40 AM, Gerald Mücke gerald.muecke@gmail.com wrote:

Hi Alex,

I will have the issues fixed, what about the new features that have no corresponding jira-ticket? do we create new tickets for those?

On Mon, Mar 2, 2015 at 11:49 PM, Alexandre Victoor < notifications@github.com> wrote:

It looks like you spend quite some time on this PR, but sorry some changes are needed on my side in order to accept it. Please reformat the code to match coding rules ( https://github.com/SonarSource/sonar-developer-toolset#code-style) Also we try to link every single change to a jira ticket. Try to link commits with jira tickets. Some commits are linked to issues (compatibility with sonarqube 5.0) and other to new features that are described nowhere. Please remove unnecessary comments in the code such as "logger: SLF4J Logger for this class"

— Reply to this email directly or view it on GitHub https://github.com/SonarCommunity/sonar-pitest/pull/4#issuecomment-76844435 .

alexvictoor commented 9 years ago

Hello again sorry for the late response I will try to integrate your pull request by the end of this wek (many things in this one) I guess our git history could be better but I will not bother you with that

gmuecke commented 9 years ago

Hi Alexandre,

no need to hurry, there is maybe an issue with multi-module projects, where the sonar analysis will fail because it does not find the resources (I have to investigate) I may fix it with the current rull-request but I'd prefer to have a separate issue (jira ticket) for it, but I have no permission to do so.

Regarding all the other commits, I could still assign those to a jira ticket, but the same problem applies here to: I'd like to create a jira "Improvement"-typed ticket, but are not allowed to do so

Whats the process of getting the proper access permissions for the sonar plugin project?

Kind regards, Gerald

On Tue, Mar 10, 2015 at 9:58 PM, Alexandre Victoor <notifications@github.com

wrote:

Hello again sorry for the late response I will try to integrate your pull request by the end of this wek (many things in this one) I guess our git history could be better but I will not bother you with that

— Reply to this email directly or view it on GitHub https://github.com/SonarCommunity/sonar-pitest/pull/4#issuecomment-78146665 .

gmuecke commented 9 years ago

It seems the problem with multi-module project occurs only on the 4.5.1 backport. So no worries, please go on integrating it as you please.

alexvictoor commented 9 years ago

Thanks I will try to integrate it by the end of this week Sorry again for the delay, I am quite busy these days...

gmuecke commented 9 years ago

Hi Alexandre,

if there's anything I can help you with for speeding up the process, let me know! At least at the moment I'm a bit less busy

Kind regards, Gerald

On Mon, Mar 16, 2015 at 8:58 PM, Alexandre Victoor <notifications@github.com

wrote:

Thanks I will try to integrate it by the end of this week Sorry again for the delay, I am quite busy these days...

— Reply to this email directly or view it on GitHub https://github.com/SonarCommunity/sonar-pitest/pull/4#issuecomment-81904692 .

alexvictoor commented 9 years ago

Hello Gerald Sorry again for the delay. I am a bit busy preparing 2 talks for devoxxfr...

alexvictoor commented 9 years ago

Gerald, Sorry again for the way too long delay. Now that devoxxfr is over, I have time and energy to work on sonar pitest. About your PR, the build is red, a test fails... I have not take a look at it, I guess it might be trivial to solve

gmuecke commented 9 years ago

Hi Alex, the test break was induces by xml reformatting done on pre-commit by intellij. Its fixed now and the code should be less prone to reformattings of the xml now

alexvictoor commented 9 years ago

Thanks Gerald I will have quite a lot of work to integrate this pull request. Please next time keep the same conventions already in place in the code. Regarding the tests, there is a convention where each test case is related to a behavior and hence begins with a "should". The body of the test is usually split in 3 sections "given", "when", "then". This does not make sens to test getters and setters.

gmuecke commented 9 years ago

Hi Alex,

sorry for making such a huge change in on PR, I started playing around with the original code and it led me sort of accidentally to where it is now :) But as it is no gradual change but more of a complete overhaul, I deliberately versioned it 1.0 and you could simply merge it as-is (but of course, thats up to you).

Regarding the convention starting tests with "should"... I usually stick to the generated test-method names and extend them with some param and expected result information where it makes sense. Is it a sonar-community convention? Could you provide me a pointer to where I can find more information about the sonar coding conventions so I'm better prepared next time?

Most of the tests should have a structure "prepare", "act", "assert" which is basically the same as given-when-then (I´m using a template for that from another project in my default configuration). I agree, testing getters and setters is a no-brainer, the reason I usually do it anyway is that I´ve seen it non-implemented returning null in production code, and its a quick win to get line coverage :)

Thanks for your effort! Gerald

On Sun, Apr 12, 2015 at 11:01 PM, Alexandre Victoor < notifications@github.com> wrote:

Thanks Gerald I will quite a lot of work to integrate this pull request. Please next time keep the same conventions already in place in the code. Regarding the tests, there is a convention where each test case is related to a behavior and hence begins with a "should". The body of the test is usually split in 3 sections "given", "when", "then". This does not make sens to test getters and setters.

— Reply to this email directly or view it on GitHub https://github.com/SonarCommunity/sonar-pitest/pull/4#issuecomment-92126670 .

tylersmith34 commented 9 years ago

Can you guys provide an update on when you might have a plugin that works with Sonar 5? I had already upgraded to a version of Sonar that doesn't work with the PIT plugin and will be willing to upgrade to Sonar v5 to get this plugin.

gmuecke commented 9 years ago

Hi Tyler,

if you can't wait, you can use the version from the PR https://github.com/gmuecke/sonar-pitest build it yourself and put it in the sonar extensions/plugin directory. I have it running on Sonar 5.0.1 (see https://sonar.inkstand.io/dashboard/index/624 on top-right)

tylersmith34 commented 9 years ago

@gmuecke Thanks, I'll do that until this release is ready.