"Outdated" comments not deleted about violation-comments-to-stash-plugin HOT 27 OPEN

When true, it will not remove identical comments. It will also keep old comments on violations that are not found.

When false, it will always remove all old comments before commenting again.

from violation-comments-to-stash-plugin.

https://github.com/tomasbjerre/violation-comments-lib/blob/master/src/main/java/se/bjurr/violations/comments/lib/CommentsCreator.java#L84

from violation-comments-to-stash-plugin.

Ok, then I have understood the setting correctly. However, old comments are not deleted.

from violation-comments-to-stash-plugin.

You have keepOldComments: false and you have several comments with identical occurrence of <this is a auto generated comment from violation-comments-lib F7F8ASD8123FSDF> <a945025467>?

from violation-comments-to-stash-plugin.

I suppose the default setting is false, as it did not come out of the Pipeline Snippet Generator or how it is called. I will try with setting it to false explicitly.

But yes, I have multiple occurrences of that.

from violation-comments-to-stash-plugin.

Ok, setting is explicitly, did not make any change:

ViolationsToBitbucketServer(
[
	bitbucketServerUrl: 'url',
	commentOnlyChangedContent: true,
	commentOnlyChangedContentContext: 50,
	commentTemplate: '',
	createSingleFileComments: true,
	credentialsId: 'credentials',
	keepOldComments: false,
	projectKey: 'project',
	pullRequestId: env.CHANGE_ID,
	repoSlug: 'code',
	violationConfigs: [
		[parser: 'CPPCHECK', pattern: '.*cppcheckResult\\.xml$', reporter: ''],
		[parser: 'CPPLINT', pattern: '.*cpplintResult\\.xml$', reporter: ''],
		[parser: 'CLANG', pattern: '.*GccOutput\\.txt$', reporter: 'Gcc'],
		[parser: 'CLANG', pattern: '.*DoxygenOutput\\.txt$', reporter: 'Doxygen']
	]
])

from violation-comments-to-stash-plugin.

Do you see any info in the global system log? https://github.com/tomasbjerre/violation-comments-to-bitbucket-server-lib/blob/master/src/main/java/se/bjurr/violations/comments/bitbucketserver/lib/BitbucketServerCommentsProvider.java#L137

from violation-comments-to-stash-plugin.

I doubt that this helps:

Jul 09, 2018 10:40:35 AM INFO org.jenkinsci.plugins.workflow.job.WorkflowRun finish
code/LIM-565-Solve-problem-with-Violation-Comments-To-Bitbucket-plugin #19 completed: ABORTED

from violation-comments-to-stash-plugin.

I observerd something new: I think it is the Bitbucket default settings to give a reviewer the chance to comment 10 lines around the changed lines. The comments are then surrounded by the code themselves in the Bitbucket overview page. Though, I configured the plugin to comment around 50 lines. Then the comments stay alone without any surrounding code snippet.

The latter comment gets "outdated" automatically when any new commit is pushed (as the one above which I posted). This "outdated" flag can be seen in the overview tab of the pull request. Then the comment gets duplicated. In contrast I have CPPCHECK comments which were in the +-10 lines boundary, did not become outdated and were not duplicated.

Maybe this "outdated" flag has something to do with the duplication. However, the cppcheck warnings are not deleted and created again, but they stay and are not created again.

I also re-run the same commit. No additional duplicates at all.

I use Bitbucket Server 5.7.1

from violation-comments-to-stash-plugin.

The outdated thing is expected. You cannot add such a comment from the GUI, just with the REST API. Probably why they chose to mark it as outdated because they assume it was added from the GUI in a previous revision...

I made some tests locally now and my comments are getting removed as expected. If I remove one comment it will re-appear after a new analysis. Also old, identical, comments are kept.

from violation-comments-to-stash-plugin.

I also re-run the whole, but this time I set commentOnlyChangedContentContext: 10.

The Cppcheck warnings vanished completely, so removal does work somehow. However, this was not really expected, because the change happened within the boundary. Does a code line removal does not cound?

The cpplint warnings - from above - were flagged as outdated, but have not been removed. However, I expected they would have been removed, because they are outside the 10 lines boundary now.

So I am convinced that the removal of comments which are flagged as outdated, does not work. Or is it expected that they stay?

from violation-comments-to-stash-plugin.

Ok I don't know about the outdated comments. Perhaps you are right about that...

Yes a code line removal should count and make violations appear within the boundary of that change.

from violation-comments-to-stash-plugin.

Ok, I had setup a new logger for warning level and got this for some comments;

Jul 09, 2018 12:51:37 PM WARNING se.bjurr.violations.comments.bitbucketserver.lib.BitbucketServerCommentsProvider removeComments

Was unable to remove comment 742 0
java.lang.NullPointerException
	at se.bjurr.violations.comments.bitbucketserver.lib.client.BitbucketServerInvoker.doInvokeUrl(BitbucketServerInvoker.java:112)
Caused: java.lang.RuntimeException: Error calling:
https://url/rest/api/1.0/projects/P2/repos/code/pull-requests/18/comments/742?version=0
DELETE
null
	at se.bjurr.violations.comments.bitbucketserver.lib.client.BitbucketServerInvoker.doInvokeUrl(BitbucketServerInvoker.java:121)
	at se.bjurr.violations.comments.bitbucketserver.lib.client.BitbucketServerInvoker.invokeUrl(BitbucketServerInvoker.java:63)
	at se.bjurr.violations.comments.bitbucketserver.lib.client.BitbucketServerClient.doInvokeUrl(BitbucketServerClient.java:104)
	at se.bjurr.violations.comments.bitbucketserver.lib.client.BitbucketServerClient.pullRequestRemoveComment(BitbucketServerClient.java:159)
	at se.bjurr.violations.comments.bitbucketserver.lib.BitbucketServerCommentsProvider.removeComments(BitbucketServerCommentsProvider.java:130)
	at se.bjurr.violations.comments.lib.CommentsCreator.removeOldCommentsThatAreNotStillReported(CommentsCreator.java:187)
	at se.bjurr.violations.comments.lib.CommentsCreator.createSingleFileComments(CommentsCreator.java:151)
	at se.bjurr.violations.comments.lib.CommentsCreator.createComments(CommentsCreator.java:67)
	at se.bjurr.violations.comments.lib.CommentsCreator.createComments(CommentsCreator.java:40)
	at se.bjurr.violations.comments.bitbucketserver.lib.ViolationCommentsToBitbucketServerApi.toPullRequest(ViolationCommentsToBitbucketServerApi.java:164)
	at org.jenkinsci.plugins.jvctb.perform.JvctbPerformer.doPerform(JvctbPerformer.java:148)
	at org.jenkinsci.plugins.jvctb.perform.JvctbPerformer$1.invoke(JvctbPerformer.java:275)
	at org.jenkinsci.plugins.jvctb.perform.JvctbPerformer$1.invoke(JvctbPerformer.java:263)
	at hudson.FilePath.act(FilePath.java:1009)
	at hudson.FilePath.act(FilePath.java:987)
	at org.jenkinsci.plugins.jvctb.perform.JvctbPerformer.jvctsPerform(JvctbPerformer.java:262)
	at org.jenkinsci.plugins.jvctb.ViolationsToBitbucketServerRecorder.perform(ViolationsToBitbucketServerRecorder.java:66)
	at org.jenkinsci.plugins.workflow.steps.CoreStep$Execution.run(CoreStep.java:80)
	at org.jenkinsci.plugins.workflow.steps.CoreStep$Execution.run(CoreStep.java:67)
	at org.jenkinsci.plugins.workflow.steps.SynchronousNonBlockingStepExecution$1$1.call(SynchronousNonBlockingStepExecution.java:50)
	at hudson.security.ACL.impersonate(ACL.java:290)
	at org.jenkinsci.plugins.workflow.steps.SynchronousNonBlockingStepExecution$1.run(SynchronousNonBlockingStepExecution.java:47)
	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
	at java.lang.Thread.run(Thread.java:748)

Disappointingly, I don't know which comment is which and I have deleted the outdated comments in the meantime.

from violation-comments-to-stash-plugin.

I noticed that as well, made a release with a fix. But this is just spamming the logs, it does not cause any other problem. The comment is removed.

from violation-comments-to-stash-plugin.

I had a look to your fix, so it prevents the exception and the logging, but it does not solve the problem, I think.

Shouldn't we get a response with a body when a comment is deleted (I have no idea)? When I try the url from the exception manually, I get a error result with "Comment 742 does not exist." So has it moved in the meantime?

from violation-comments-to-stash-plugin.

No there is no response to parse when delete is invoked. So I think it is correct fix to not try to parse anything.

And when you try the url manually is is already deleted by the plugin.

from violation-comments-to-stash-plugin.

AFAICR it's not possible to delete the outdated comments you see in the overview tab.

from violation-comments-to-stash-plugin.

@jansohn You mean from REST API, because from GUI it is possible.

from violation-comments-to-stash-plugin.

I don't see a Delete link in the GUI either (for OUTDATED comments).

from violation-comments-to-stash-plugin.

For me, this is possible, maybe a kind of system setting or branch permission or so.

from violation-comments-to-stash-plugin.

I think I can explain why the outdated comments are not deleted.

If path is not specified when asking the REST API for comments:
http://localhost:7990/rest/api/latest/projects/PROJECT_1/repos/violations-test/pull-requests/1/comments
Then the response is:

{"errors":[{"context":null,"message":"The path query parameter is required when retrieving comments.","exceptionName":null}]}

So a path parameter is needed. Which is why the library first asks for all changed files, and then for the comments on each changed file:

  @Override
  public List<Comment> getComments() {
    final List<Comment> comments = newArrayList();
    for (final String changedFile : client.pullRequestChanges()) {
      final List<BitbucketServerComment> bitbucketServerCommentsOnFile =
          client.pullRequestComments(changedFile);
      for (final BitbucketServerComment fileComment : bitbucketServerCommentsOnFile) {
        final List<String> specifics = newArrayList(fileComment.getVersion() + "", changedFile);
        comments.add(new Comment(fileComment.getId() + "", fileComment.getText(), null, specifics));
      }
    }

    return comments;
  }

In my case, to get outdated comments, I reverted my changes in a file. Pushed that commit. And Bitbucket Shows comments on that file as outdated.

But then, since the file is no longer "changed", the comments on it will not be requested, will not be found and will not be deleted...

from violation-comments-to-stash-plugin.

This explains certainly a part of the problem. I have also reverted the changes here and there so this explains why comments are not removed after a change has been reverted and the file is unchanged in comparison to the master then.

But try this as well:

• Set commentOnlyChangedContentContext: 50 or something other higher than the Bitbucket default of 10.
• Provoke a finding in the near of a change between 11 and 50 lines, so that it is not shown in the overview with a code snippet around it.
• Execute build, finding is found and shown in overview tab.
• Change that file at a different place and push again.
• The comment should become outdated directly from the beginning of the build. My observation is that it is not removed and a new comment will be created. This is how duplication shows up.

from violation-comments-to-stash-plugin.

I think most users have a context of around 10. I think having 50 might spam developers with findings that they did not introduce. And it is my experience that developers don't want that. My point is that I suspect this is a minor issue if people use a low context, what do you think?

from violation-comments-to-stash-plugin.

I agree with your opinion.

My thought was to improve the legacy code and not just to not introduce further problems. But this is highly discussible. Thus I accept that this is a minor bug. I also returned to put the context to 10 as workaround.

from violation-comments-to-stash-plugin.

We have the same problem. If the plugin creates an comment on for a specific line die comment shows up in bitbucket. After the developer fixes the error the comment is shown as outdated but is not deleted.
The keepOldComments flag is set to false. I used the latest version of the plugin. We had set the commentOnlyChangedContentContext = 0.

from violation-comments-to-stash-plugin.

That probably depends on how the violation was fixed. If it is fixed by reverting the changes to that line, then I can see how it is outdated. But if the line is still changed after the fix, then I don't think this can happen.

from violation-comments-to-stash-plugin.

The line is new and only changed during a second commit. but the comment still exists.

from violation-comments-to-stash-plugin.