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

Added a sourcesPaths array tag that will add multiple Sources to the … #43

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

pfrank13
Copy link

…Cobertura ArgumentBuilder.

E.g.

${project.basedir}/src/main/groovy/
${project.basedir}/src/main/java/

Adds support for both groovy files and java files in a GMavenPlus Plugin default layout.

…Cobertura ArgumentBuilder.

E.g.
            <sourcesPaths>
              <scourcesPath>${project.basedir}/src/main/groovy/</scourcesPath>
              <scourcesPath>${project.basedir}/src/main/java/</scourcesPath>
            </sourcesPaths>
Adds support for both groovy files and java files in a GMavenPlus Plugin default layout.
Peter Frank added 5 commits May 22, 2017 15:29
This isn't wired into the runtime of the project yet but the problem we have is Cobertura doesn't reference the original sources directory for multiple source sets in it's XML output.  That being said, in the ProjectData model object we get the filename in the form "my/package/MyClass.java" from the ClassData.getSourceFileName() which we can use as the basis for testing all sourcesPaths to see if in which one a given file exists.  With that then we can generate a proper Sonarqube XML representation by just rendering out the JAXB objects to the sonarqube xml output file.
Created the CoberturaToSonarQubeReportConverter abstraction to provide a seam for using the XSL way of transformation in addition to the Jaxb way of transformation
…nverter

Just put in a hardcoded version of JaxbCoberturaToSonarQubeReportConverter to test that it will work within what exists in the existing plugin flows
The XSD wasn't strictly correct as there was a cardinality mismatch between CoverateType and FileType, ti needed to be 1 to many, not 1 to 1
Just beginning implementaiton of the JaxbCoberturaToSonarQubeReportConverter
Now the Sonar report generation is covered for supporting multiple sourcesPaths, it also works with a single sourcesPath as well to not break backwards compatibility.  There is still current a way to go back to the XSL transformation but that will for sure generate some nonsense in the multiple sourcesPaths case and should be removed before merging into master
@pfrank13
Copy link
Author

@pawlakm, so this is functionally complete w/support for the sonar xml file generation with both a single sourcesPath and the new array sourcesPaths. Take a look when you have a chance.

Also, there is a test harness artifact in there that should be removed but figured I'd leave it for you to be able to easily switch from the old XSL transformation to the JAXB transformation logic.
Specifically this field

    //TODO remove this before merging, it's just for testing purposes in case
    @Parameter(defaultValue = "false", required = false)
    private boolean useXslTransform;

@@ -75,6 +78,12 @@ public void convertReport(
final String[] sourcesPaths,
final String projectPath) throws MojoExecutionException {
try {
//There is no real lookup but the project doesn't have Tuple or KeyValuePair
Copy link
Author

Choose a reason for hiding this comment

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

Small smell here, using in a Map in not a very map like way, e.g. I only iterate over the EntrySet and don't do any get('key').

if(fileToCheck.exists()){
return sonarSourcesPathPrefixEntry.getValue();
}else {
//Should we even bother defaulting?
Copy link
Author

Choose a reason for hiding this comment

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

The entire defaulting thing here can potentially be removed and just returning a default "" and just know that the file name is not going to resolve because we don't know what it is.

@pfrank13
Copy link
Author

Just pinging to see if there is anything i can do to help this be accepted.

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

Successfully merging this pull request may close these issues.

None yet

1 participant