Skip to content
This repository has been archived by the owner on Jan 30, 2023. It is now read-only.

WIP: Upgrade to SonarQube 7.6 #152

Merged
merged 18 commits into from Mar 10, 2019

Conversation

BalmungSan
Copy link
Contributor

@BalmungSan BalmungSan commented Feb 15, 2019

Good news: Everything compiles 馃帀
Bad news: A couple of deprecation warnings! 馃檧 - Apparently they removed the concept of modules, thus the changes may be heavy, I have not looked to them yet, hopping to start working in this latter today, or tomorrow.

Anyways, at this moment I can not test this with a real SonarQube server.
Maybe @manodupont or @stillleben can help out with that part.
It would be good to test now and after removing the deprecations - on the worst case, if I can not fix them in the short term, we may merge this as is, release (if everything works) and fix the deprecations latter.


Edit

Real bad new!: I only checked if it compiled, but did not run the tests. 馃槥 - Apparently, since the tests uses the internal API those are broken (don't even compile). Sorry about that.

@mwz
Copy link
Member

mwz commented Feb 15, 2019

Thanks for looking into this @BalmungSan - perhaps it would be easier to try to upgrade to 7.5 first?

@BalmungSan
Copy link
Contributor Author

"Perhaps it would be easier to try to upgrade to 7.5 first?"

May not be 100% true, but I think not.
The release notes state that 7.5 only added support for Scala and Apex, and some other graphical changes. Thus, I do not think it will make any impact.

馃憤

@mwz
Copy link
Member

mwz commented Feb 15, 2019

Ok no problem - let me know if you need any help with this and I can give it a go if it turns out that it's too much work.

@BalmungSan
Copy link
Contributor Author

Hi @mwz, addressed the deprecations and fixed the tests...

However, I still believe this will not work... at least not for multi-module projects.
At this moment I can not run the examples against a running SonarQube server, could you help me with that?

I had been searching and Scoverage supports generation of aggregated metrics (also the sunfire maven plugin), however I can not say the same for Scapegoat and Scalastyle 馃樋 ...
Maybe, we could try to aggregate the reports ourselves?
Other option, would be to check if using the old Sensor instead of the new ProjectSensor will made them work on modules again, and see if SonarQube will be able to aggregate the metrics. Can you help me testing this one too?

PS: Also, there are a lot of uses of the word module on tests folder, I will try to fix that tomorrow.

@mwz
Copy link
Member

mwz commented Feb 22, 2019

Sorry, @BalmungSan I haven't yet had a chance to look into this, but hopefully, I should be able to spend some time on this over the weekend.

@BalmungSan
Copy link
Contributor Author

@mwz no problem, my week has been very busy too. I would not had been able to address any feedback 馃憤

log.debug(s"Input test files: \n${inputFiles.mkString(", ")}")
else
log.warn(s"No test files found for module ${context.module.key}.")

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think we don't need this anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, sorry. Forgot to ask, and end up deleting it anyways.
I did not understand what was the intended use of that.
The inputFile are only used for the logs, and I remember there were some similar logs, like empty test directories.

Care to explain what was the use of this?, If you consider it important I can add it again.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mwz So, should I restore this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think so. This will log warnings when we don't find any test files in the module, so it's useful - a gentle reminder that you should have tests in your project ;)

@mwz
Copy link
Member

mwz commented Feb 24, 2019

I've tried executing an analysis against the plugin built from your branch, but I'm getting the following dependency injection error at runtime:

ERROR: Error during SonarQube Scanner execution
org.picocontainer.injectors.AbstractInjector$UnsatisfiableDependenciesException: com.mwz.sonar.scala.scalastyle.ScalastyleSensor has unsatisfied dependency 'class org.sonar.api.profiles.RulesProfile' for constructor 'public com.mwz.sonar.scala.scalastyle.ScalastyleSensor(org.sonar.api.profiles.RulesProfile,com.mwz.sonar.scala.scalastyle.ScalastyleCheckerAPI)' from org.sonar.core.platform.ComponentContainer$ExtendedDefaultPicoContainer@740fb309:307<[Immutable]:org.sonar.core.platform.ComponentContainer$ExtendedDefaultPicoContainer@771158fb:33<|
	at org.picocontainer.injectors.ConstructorInjector.getGreediestSatisfiableConstructor(ConstructorInjector.java:191)
	at org.picocontainer.injectors.ConstructorInjector.getGreediestSatisfiableConstructor(ConstructorInjector.java:110)
	at org.picocontainer.injectors.ConstructorInjector.access$100(ConstructorInjector.java:51)
	at org.picocontainer.injectors.ConstructorInjector$1.run(ConstructorInjector.java:331)
	at org.picocontainer.injectors.AbstractInjector$ThreadLocalCyclicDependencyGuard.observe(AbstractInjector.java:270)
	at org.picocontainer.injectors.ConstructorInjector.getComponentInstance(ConstructorInjector.java:364)
	at org.picocontainer.injectors.AbstractInjectionFactory$LifecycleAdapter.getComponentInstance(AbstractInjectionFactory.java:56)
	at org.picocontainer.behaviors.AbstractBehavior.getComponentInstance(AbstractBehavior.java:64)
	at org.picocontainer.behaviors.Stored.getComponentInstance(Stored.java:91)
	at org.picocontainer.DefaultPicoContainer.getLocalInstance(DefaultPicoContainer.java:606)
	at org.picocontainer.DefaultPicoContainer.getComponents(DefaultPicoContainer.java:587)
	at org.sonar.core.platform.ComponentContainer.getComponentsByType(ComponentContainer.java:290)
	at org.sonar.scanner.bootstrap.AbstractExtensionDictionnary.completeScannerExtensions(AbstractExtensionDictionnary.java:82)
	at org.sonar.scanner.bootstrap.AbstractExtensionDictionnary.getExtensions(AbstractExtensionDictionnary.java:77)
	at org.sonar.scanner.bootstrap.AbstractExtensionDictionnary.getFilteredExtensions(AbstractExtensionDictionnary.java:67)
	at org.sonar.scanner.sensor.ProjectSensorExtensionDictionnary.selectSensors(ProjectSensorExtensionDictionnary.java:41)
	at org.sonar.scanner.sensor.ProjectSensorsExecutor.execute(ProjectSensorsExecutor.java:41)
	at org.sonar.scanner.scan.ProjectScanContainer.doAfterStart(ProjectScanContainer.java:363)
	at org.sonar.core.platform.ComponentContainer.startComponents(ComponentContainer.java:136)
	at org.sonar.core.platform.ComponentContainer.execute(ComponentContainer.java:122)
	at org.sonar.scanner.bootstrap.GlobalContainer.doAfterStart(GlobalContainer.java:126)
	at org.sonar.core.platform.ComponentContainer.startComponents(ComponentContainer.java:136)
	at org.sonar.core.platform.ComponentContainer.execute(ComponentContainer.java:122)
	at org.sonar.batch.bootstrapper.Batch.doExecute(Batch.java:73)
	at org.sonar.batch.bootstrapper.Batch.executeTask(Batch.java:99)
	at org.sonarsource.scanner.api.internal.batch.BatchIsolatedLauncher.execute(BatchIsolatedLauncher.java:63)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.sonarsource.scanner.api.internal.IsolatedLauncherProxy.invoke(IsolatedLauncherProxy.java:60)
	at com.sun.proxy.$Proxy0.execute(Unknown Source)
	at org.sonarsource.scanner.api.EmbeddedScanner.doExecute(EmbeddedScanner.java:233)
	at org.sonarsource.scanner.api.EmbeddedScanner.runAnalysis(EmbeddedScanner.java:151)
	at org.sonarsource.scanner.cli.Main.runAnalysis(Main.java:123)
	at org.sonarsource.scanner.cli.Main.execute(Main.java:77)
	at org.sonarsource.scanner.cli.Main.main(Main.java:61)

I think that's caused by the RulesProfile no longer being available for scanner side components (as hinted in the API changes the ActiveRules should be used instead)

@mwz
Copy link
Member

mwz commented Feb 24, 2019

So I had a brief look at the recent SQ changes and it sounds to me that they removed the module functionality from the UI and deprecated some of the batch APIs, but everything should work for now (except for the removed APIs for RulesProfile which is what we use and scanner tasks which we don't use).

I think we should still try to scan using modules because as far as I understand this functionality should still work along with scanner compatibility and while they are phasing modules out we can come up with a longer term plan on how we can do aggregation on our end (if it's necessary and not taken care by the "new scanner engine").

I'm going to test against 7.6 with those deprecated APIs and the latest scanner to check if things are still working - the best case scenario is we'll only need to fix the RulesProfile breaking change.

@BalmungSan
Copy link
Contributor Author

Hi, will try to finish this and fix the dependency injection problem today or tomorrow.

"The best case scenario is we'll only need to fix the RulesProfile breaking change".

I hope so - will stay tuned for your update.

"I think we should still try to scan using modules because as far as I understand this functionality should still work along with scanner compatibility".

If it works, I agree we should do so for Scapegoat and Scalastyle, but if Scoverage & JUnit already support generation of aggregated reports, I think (IMHO) we should prefer that. - What do you think.

@mwz
Copy link
Member

mwz commented Feb 25, 2019

I've made the necessary changes on a separate branch (see here) and everything seems to be working fine although it might require some more testing as I've checked only the single and multi-module example projects. (I haven't fixed Scapegoat tests, but I did that for Scalastyle).

Feel free to use my branch as a reference.

These are the only warnings I get from the scanner:

WARN: Storing measures on folders or modules is deprecated. Provided value of metric 'sonar-scala-scoverage-total-statements' is ignored.
WARN: Storing measures on folders or modules is deprecated. Provided value of metric 'sonar-scala-scoverage-covered-statements' is ignored.
WARN: Storing measures on folders or modules is deprecated. Provided value of metric 'sonar-scala-scoverage-statement-coverage' is ignored.
WARN: Storing measures on folders or modules is deprecated. Provided value of metric 'sonar-scala-scoverage-branch-scoverage' is ignored.

I don't think we need to worry about those for now - I've checked and all the coverage metrics are saved correctly. I'm not sure why we're getting those errors - I don't think we save any of those metrics on either module or folders as we do that on a file level... Maybe it's something we can look into later.

Hi, will try to finish this and fix the dependency injection problem today or tomorrow.

Looks like we've used RulesProfile to get the name of the default quality profile (here) so I got rid of it entirely.

If it works, I agree we should do so for Scapegoat and Scalastyle, but if Scoverage & JUnit already support generation of aggregated reports, I think (IMHO) we should prefer that. - What do you think.

I'd generally agree with that, however we'd need to be careful not to confuse people as to which features use aggregated reports and which don't. It would be great if we could be consistent and base everything on aggregated reports, but as you said before Scapegoat doesn't support this, so most likely to implement a project level sensor we would need users to define source directories of their modules in sonar.sources property (sbt-sonar could do that), generate scapegoat report for each module and during the analysis we would load and process all of the available reports and maybe for consistency reasons we should do the same with scoverage and junit. It would be nice as a user not having to worry about aggregation and run any additional tasks. With Scalastyle we have a lot more controll because we can just run it for each directory with sources, so that sould be fairly straightforward.

We should definitely discuss it in more details and agree on the approach for next versions. For 7.6 though I'd be happy to go with those deprecated APIs and a minimal set of changes given that everything still works.

@mwz
Copy link
Member

mwz commented Mar 1, 2019

Hey @BalmungSan, let me know if you want me to pick this up.

@BalmungSan
Copy link
Contributor Author

Hi @mwz sorry, this week was even worse than the last one. I hope everything will finish soon and after that I will have some spare time, I really had not even read your previous comment... 馃槥

I should be able to resume this before Monday, but if you wan to improve something meanwhile or close this PR and open one yourself, I would be fine.

@mwz
Copy link
Member

mwz commented Mar 2, 2019

I should be able to resume this before Monday, but if you wan to improve something meanwhile or close this PR and open one yourself, I would be fine.

Ok, no worries - I'll leave this to you then.

@BalmungSan
Copy link
Contributor Author

BalmungSan commented Mar 2, 2019

"We should definitely discuss it in more details and agree on the approach for next versions. For 7.6 though I'd be happy to go with those deprecated APIs and a minimal set of changes given that everything still works".

I agree we should discuss this in more detailed way latter (probably we should open a new Issue for tracking that #165 ).

I somewhat disagree with your idea, but I'll keep my thoughts for now, and we will discuss it latter 馃槈
For now I will leave everything working in a module level, but will remove everything that says "module".

Are you ok with that?

@BalmungSan
Copy link
Contributor Author

Hi @mwz, I think I am done here. 馃
let me know if everything works as expected or if you have any suggestion.

After this gets merged, I will post in #165 my thoughts about how I believe the plugin should work on future releases.

@mwz
Copy link
Member

mwz commented Mar 4, 2019

Thanks @BalmungSan, the changes look good to me. I'll test this locally and if everything is good I'll merge it and make a release.

@mwz
Copy link
Member

mwz commented Mar 6, 2019

I've just tested this against the example multi-module sbt project and this is the error I got during the analysis of a second module:

ERROR: Error during SonarQube Scanner execution
java.lang.UnsupportedOperationException: Can not add the same measure twice on [key=example-sbt-multi-module]: DefaultMeasure[component=[key=example-sbt-multi-module],metric=Metric[id=,key=sonar-scala-scoverage-total-statements,description=Number of all statements,type=INT,direction=1,domain=Size,name=Total statements,qualitative=false,userManaged=false,enabled=true,worstValue=,bestValue=,optimizedBestValue=false,hidden=false,deleteHistoricalData=false,decimalScale=],value=17,fromCore=false]
at org.sonar.scanner.sensor.DefaultSensorStorage.saveMeasure(DefaultSensorStorage.java:298)
at org.sonar.scanner.sensor.DefaultSensorStorage.store(DefaultSensorStorage.java:236)
at org.sonar.api.batch.sensor.measure.internal.DefaultMeasure.doSave(DefaultMeasure.java:95)
at org.sonar.api.batch.sensor.internal.DefaultStorable.save(DefaultStorable.java:45)
at com.mwz.sonar.scala.scoverage.ScoverageSensor.saveComponentScoverage(ScoverageSensor.scala:136)
at com.mwz.sonar.scala.scoverage.ScoverageSensor.execute(ScoverageSensor.scala:67)
at org.sonar.scanner.sensor.AbstractSensorWrapper.analyse(AbstractSensorWrapper.java:48)
at org.sonar.scanner.sensor.ModuleSensorsExecutor.execute(ModuleSensorsExecutor.java:85)
at org.sonar.scanner.sensor.ModuleSensorsExecutor.lambda$execute$1(ModuleSensorsExecutor.java:59)
at org.sonar.scanner.sensor.ModuleSensorsExecutor.withModuleStrategy(ModuleSensorsExecutor.java:77)
at org.sonar.scanner.sensor.ModuleSensorsExecutor.execute(ModuleSensorsExecutor.java:59)
at org.sonar.scanner.scan.ModuleScanContainer.doAfterStart(ModuleScanContainer.java:82)
at org.sonar.core.platform.ComponentContainer.startComponents(ComponentContainer.java:136)
at org.sonar.core.platform.ComponentContainer.execute(ComponentContainer.java:122)
at org.sonar.scanner.scan.ProjectScanContainer.scan(ProjectScanContainer.java:408)
at org.sonar.scanner.scan.ProjectScanContainer.scanRecursively(ProjectScanContainer.java:403)
at org.sonar.scanner.scan.ProjectScanContainer.scanRecursively(ProjectScanContainer.java:400)
at org.sonar.scanner.scan.ProjectScanContainer.doAfterStart(ProjectScanContainer.java:360)
at org.sonar.core.platform.ComponentContainer.startComponents(ComponentContainer.java:136)
at org.sonar.core.platform.ComponentContainer.execute(ComponentContainer.java:122)
at org.sonar.scanner.bootstrap.GlobalContainer.doAfterStart(GlobalContainer.java:126)
at org.sonar.core.platform.ComponentContainer.startComponents(ComponentContainer.java:136)
at org.sonar.core.platform.ComponentContainer.execute(ComponentContainer.java:122)
at org.sonar.batch.bootstrapper.Batch.doExecute(Batch.java:73)
at org.sonar.batch.bootstrapper.Batch.executeTask(Batch.java:99)
at org.sonarsource.scanner.api.internal.batch.BatchIsolatedLauncher.execute(BatchIsolatedLauncher.java:63)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at org.sonarsource.scanner.api.internal.IsolatedLauncherProxy.invoke(IsolatedLauncherProxy.java:60)
at com.sun.proxy.$Proxy0.execute(Unknown Source)
at org.sonarsource.scanner.api.EmbeddedScanner.doExecute(EmbeddedScanner.java:233)
at org.sonarsource.scanner.api.EmbeddedScanner.runAnalysis(EmbeddedScanner.java:151)
at org.sonarsource.scanner.cli.Main.runAnalysis(Main.java:123)
at org.sonarsource.scanner.cli.Main.execute(Main.java:77)
at org.sonarsource.scanner.cli.Main.main(Main.java:61)


saveComponentScoverage(context, context.module(), moduleCoverage.moduleScoverage)
saveComponentScoverage(context, context.project, projectCoverage.projectScoverage)
Copy link
Member

@mwz mwz Mar 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might still need to save this against the current module, otherwise the scanner explodes.

Copy link
Contributor Author

@BalmungSan BalmungSan Mar 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uhm.. the problem is that module is deprecated and the replacement is project.
And, I am not talking about the big picture about module vs project analysis, but about the context methods...

And I thought I understood that using the old context would be enough...
Because for the plugin each module would be seen as a project and that the server will aggregate the metrics...

But, apparently, I was wrong.
Can you please check if using the deprecated module solves this problem?
Other option would be to remove module metrics, and only register the files.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

module is deprecated but it doesn't mean that it doesn't work any more - it will be going away at some point but it still works fine in 7.6. I tested the changes on my branch here and as you can see from the diff I didn't change any module to project.

Copy link
Contributor Author

@BalmungSan BalmungSan Mar 7, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mwz Oh yeah, I know it still works (that is the propose of deprecating things anyways lol).

I mean that, IMO (or what I had understood from the docs), the idea was that if we used the old (almost deprecated) Plugin, instead of the new ProjectPlugin, the project would be the module.

But apparently it does not. - And, since if you said it works, I will revert the changes.

Do you want me to revert it everywhere (a couple of logs)?, or just there?

Also, since the are no more "modules" at interface level, how are these metrics displayed? - Maybe, they just get lost? That is why I suggested just removing the saveMetrics step for the modoule / project (or whatever) until this gets solved.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And, since if you said it works, I will revert the changes.

Do you want me to revert it everywhere (a couple of logs)?, or just there?

Great, thanks. I don't mind the logs that much, I'm ok to leave them as they are.

Also, since the are no more "modules" at interface level, how are these metrics displayed?

I'm not sure what happens to those - last time I tested it I might have seen some warning regarding those - I'll check again. I though those contributed to the overall metrics for the whole projects but it might not be the case anymore.

Copy link
Contributor Author

@BalmungSan BalmungSan Mar 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. So for now, I will just revert the changes here.
Will commit and push in a couple of minutes.

Done!
Hope this time everything works.

@mwz
Copy link
Member

mwz commented Mar 10, 2019

Ok, I'll merge this and check the scanner warnings for module metrics tomorrow - maybe, as you said, we can remove them if they are not used anywhere.

@mwz mwz merged commit c3d5f8a into sonar-scala:master Mar 10, 2019
@BalmungSan BalmungSan deleted the upgrade-to-sonarqube-7.6 branch March 10, 2019 01:44
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants