kaspersorensen / dotnet-maven-plugin

A Maven plugin for building dotnet projects based on project.json
Apache License 2.0
25 stars 15 forks source link

Add parameter to allow for output xml flag #8

Closed zrochler closed 7 years ago

zrochler commented 7 years ago

A small change to output xml from dotnet test so build tools can be configured to look for it.

kaspersorensen commented 7 years ago

Thanks for the PR, just a bit of cosmetic stuff to improve and I think it's a good idea.

zrochler commented 7 years ago

I'm not sure there's a good place for the file to go that would consistently be in a user's gitignore (target/ isn't common in .NET, and bin/ and obj/ make me squirm), but I think you're right about not just creating files. Would it make more sense to make it optional and only output the file if it's actually requested?

(Warning: stupid java question) And if so is there a better way than

if (isTestRunnable(subDirectory)) {
    if (outputXml != null) {
        helper.executeCommand(subDirectory, "dotnet", "test", "-c", helper.getBuildConfiguration(), "-xml", outputXml);
    }
    else {
        helper.executeCommand(subDirectory, "dotnet", "test", "-c", helper.getBuildConfiguration());
    }
}
kaspersorensen commented 7 years ago

Yes we can try it like that. I would maybe suggest target (or rather, maven's "project.build.directory" which can be configured, but is usually "target") just because it is a maven dotnet plugin. But at the same time it seems like a good idea simply to make the parameter the file path - so if it is not specified it means "no test results XML wanted".

As for the arguments to executeCommand: You could create a list, build it according to your if statements and then pass the list to the method. Unfortunately there's not a "ternary array element inclusion" syntax that I am aware of.

zrochler commented 7 years ago

I removed the default for the parameter and added it (with the project.build.directory) to the all-configuration properties.

Also, I don't know if should be necessary or if it's right (if not I can revert), but I updated the version in all of the poms because I wouldn't let me run them otherwise.

kaspersorensen commented 7 years ago

LGTM!

kaspersorensen commented 7 years ago

Released as version 0.11.