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

Vote: default behavior invalid issues and messages #758

Closed
guwirth opened this issue Jan 16, 2016 · 10 comments
Closed

Vote: default behavior invalid issues and messages #758

guwirth opened this issue Jan 16, 2016 · 10 comments
Assignees
Milestone

Comments

@guwirth
Copy link
Collaborator

guwirth commented Jan 16, 2016

To speed up analysis the different reports could be generated on different workstation. One is doing the static code analysis, another one the unit tests and a third one the coverage measurement and so one. To forward the results there are two possibilities then:

  • Wait unit all results for a revision are available and forward them all together to SonarQube. This is the safe way and everything fits together. In case there is an issue or measure where no sources are available this is for sure an error. Drawback of this method is that you have to wait always until the last report is available.
  • Second possibility is to run the different analysis asynchronous, collect them all in a folder and use always the latest results only. Every time if there is at least one new report available a new SonarQube update can start. This method provides typically much faster results but with the disadvantage that there could be temporary invalid measures and issues because machines are working on different revision numbers. In this case it must be possible to ignore invalid issues and measures and continue. (See also add information when measure fails to save #757)
  • Third possibility is to mix this two approaches: do a complete new analysis once a week (e.g. over the weekend) and only incremental analysis during the week. For the switch from one mode to the other it must be ensured that all reports has been generated at least once otherwise you could run into problems like loosing false positives.

To provide a generic solution the plugin should support configuration settings like:

  • ignore invalid issues and metrics and continue
  • break if no report is available

I'm not sure if these are global settings or each sensor should have this? Solution of PR #757 is a partial solution doing this for code coverage.

What is your experience with this? Are there any other use cases?
Looking forward to get your feedback.

@guwirth guwirth changed the title Problems with distributed analysis Vote: default behavior invalid issues and messages Jan 18, 2016
@guwirth
Copy link
Collaborator Author

guwirth commented Jan 18, 2016

How should the plugin behave, please vote (see also discussion in #757):

(1) invalid issue in a report (e.g. cppcheck, external, ...)
a) create an error and stop analysis
b) create an error and continue analysis
c) create a warning and continue analysis

(2) invalid measure in a report (e.g. unit tests, coverage, ...)
a) create an error and stop analysis
b) create an error and continue analysis
c) create a warning and continue analysis

(3) no report available (configuration file contains a property but no file found)
a) create an error and stop analysis
b) create an error and continue analysis
c) create a warning and continue analysis

@jmecosta
Copy link
Member

We are voting for default behaviour.

  1. a
  2. a
  3. a if report is not one of the *.xml

On Mon, Jan 18, 2016, 18:40 Günter Wirth [email protected] wrote:

How should the plugin behave, please vote (see also discussion in #757
#757):

(1) invalid issue in a report (e.g. cppcheck, external, ...)
a) create an error and stop analysis
b) create an error and continue analysis
c) create a warning and continue analysis

(2) invalid measure in a report (e.g. unit tests, coverage, ...)
a) create an error and stop analysis
b) create an error and continue analysis
c) create a warning and continue analysis

(3) no report available (configuration file contains a property but no
file found)
a) create an error and stop analysis
b) create an error and continue analysis
c) create a warning and continue analysis


Reply to this email directly or view it on GitHub
#758 (comment)
.

@Bertk
Copy link
Contributor

Bertk commented Jan 18, 2016

My vote is:

  1. b (there are also tooling issues and stopping the analysis will might mike a tool useless. I will not modify the source to adapt it for any tool)
  2. b (another known problem are the number of lines for reported issues from PC-lint. The tool adds always one line to the last line of a block. This is not required by the C++ standard but by the C standard. We fixed several files and added an empty line and error messages were helpful to identify the files.)
  3. c (We use only full names without path and the report files are located in the source folder e.g. cppcheck-report.xml but this file will not be available for CUDA only projects and I prefer to use generic settings for multi-module projects)

=> I think we need configuration items to define the behavior

@guwirth
Copy link
Collaborator Author

guwirth commented Jan 30, 2016

Still have some questions to this:

  • understood we need mode strict and tolerant
  • question is what should be the default? strict has the advantage that configuration must fit before there are results. For beginners this could also be a disadvantage because the see nothing until everything is right.
  • strict
    • if there is no report found for a settings.conf item => error
    • if a file in a report is not found => error
    • if there is a invalid line number in a report => error
    • question is only if this makes sense in a multi-module setup, especially if all reports are in one root folder?
  • tolerant seems to be easy creates in all cases only warnings and continue

@guwirth
Copy link
Collaborator Author

guwirth commented Oct 3, 2016

Proposal for switch strictand tolerant:
use already existing sonar.cxx.errorRecoveryEnabled=True for tolerant.

@jmecosta
Copy link
Member

jmecosta commented Oct 3, 2016

@guwirth i think we should have a different property for this, since parsing c++ is different than the coverage. and users might want to tailor it accordingly. #952 implements this scenario

@guwirth
Copy link
Collaborator Author

guwirth commented Oct 7, 2016

Partly solved with #959 (invalid coverage reports)

@guwirth guwirth added this to the 0.9.7 milestone Oct 17, 2016
@guwirth
Copy link
Collaborator Author

guwirth commented Oct 17, 2016

Hi @jmecosta maybe you can also do the last step to close this issue:

Thanks in advance.

Regards,
Günter

@jmecosta
Copy link
Member

Yep, supose I can take a look.

On Mon, 17 Oct 2016 13:48 Günter Wirth, [email protected] wrote:

Hi @jmecosta https://github.com/jmecosta maybe you can also do the last
step to close this issue:

Thanks in advance.

Regards,
Günter


You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
#758 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AA_jyHVSewDCCT8rpWD3zQrrbVXB70yhks5q01KGgaJpZM4HGOw2
.

@jmecosta
Copy link
Member

@guwirth #985 covers this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants