Workarounds for incomplete tool settings

[jrnail23]

One of the difficulties I've run into with Cake is when using tools, often the tool author has not included support for all options/switches that are available for command line execution.

When I want to use such a switch, I either have to invoke the tool using a raw process, or I have to submit a PR to support the switch and wait until the next release to use it.

Perhaps we can build in a way to allow devs to include raw command line args (in addition to using the supported settings properties), so we can more easily work around missing settings until they get added.

I'm thinking of something along the lines of what @patriksvensson did in #470, where he added a base settings class with the tool path. In this case, I'm thinking maybe we add a property to that base class for raw command line args, and somehow force the tool runner to include the raw args.

I'm sure others can think of better solutions as well.

Ideas? Thoughts?

[devlead]

One way would be an optional pre execute Func.

[patriksvensson]

I think that this is a good idea. We only need to make it clear that
setting this overrides all other settings.
On Nov 6, 2015 18:32, "James Nail" notifications@github.com wrote:

One of the difficulties I've run into with Cake is when using tools, often
the tool author has not included support for all options/switches that are
available for command line execution.

When I want to use such a switch, I either have to invoke the tool using a
raw process, or I have to submit a PR to support the switch and wait until
the next release to use it.

Perhaps we can build in a way to allow devs to include raw command line
args (in addition to using the supported settings properties), so we can
more easily work around missing settings until they get added.

I'm thinking of something along the lines of what @patriksvensson
https://github.com/patriksvensson did in #470
#470, where he added a base
settings class with the tool path. In this case, I'm thinking maybe we add
a property to that base class for raw command line args, and somehow force
the tool runner to include the raw args.

I'm sure others can think of better solutions as well.

Ideas? Thoughts?

—
Reply to this email directly or view it on GitHub
#500.

[patriksvensson]

@devlead Why not a setting on the ToolSettings class. If it's used then use that one instead of the arguments provided by the tools.

[devlead]

Wouldn't need to override per say, could be more of a "patch" where you get incoming args/settings and pass them back as return value from the delegate.

[jrnail23]

@patriksvensson, yes, it needs to be clear that this is a brute force approach. Although I wouldn't necessarily want to make this the override all the command line args from settings -- we should still use the settings, but add the raw args to augment them.

[devlead]

Yes it would make sense on the tool settings class, but wouldn't want to add all arguments if it's just 1 I'm missing.

[jrnail23]

@devlead, that's exactly my feeling as well.

So right now, we pretty much all construct our command line args inside the runner, right?
Would it make sense to move that responsibility into the settings class itself?

Or perhaps into a different type (maybe something like an ArgSerializer, etc.) that's focused solely on that responsibility -- that way we could inherit from the one that's built in and customize it for such workarounds. Aliases could accept this type as an overload / optional arg, where the default implementation would be used if not provided.

[jrnail23]

Here's what I'm trying right now, as a way to use a non-supported switch (-hideskipped) in the new OpenCover tool:

    public class CustomizableProcessRunner : IProcessRunner
    {
        private readonly IProcessRunner _inner;
        private readonly Action<ProcessArgumentBuilder> _argsCustomization;

        public CustomizableProcessRunner(IProcessRunner inner,
            Action<ProcessArgumentBuilder> argsCustomization)
        {
            if (inner == null)
            {
                throw new ArgumentNullException("inner");
            }
            if (argsCustomization == null)
            {
                throw new ArgumentNullException("argsCustomization");
            }

            _inner = inner;
            _argsCustomization = argsCustomization;
        }

        IProcess IProcessRunner.Start(FilePath filePath, ProcessSettings settings)
        {
            _argsCustomization(settings.Arguments);

            return _inner.Start(filePath, settings);
        }
    }

// ... in my script:

var runner = new CustomizableProcessRunner(cc.ProcessRunner, args => args.Append("-hideskipped:All"));

new OpenCoverRunner(cc.FileSystem, cc.Environment, runner, cc.Globber)
    .Run(cc, tool => tool.NUnit(unitTestAssemblies,
        new NUnitSettings
        {
            Framework = "net-4.0",
            NoLogo = true,
            ResultsFile = testResultsFile,
            ShadowCopy = false,
            Timeout = 5000 //(ms)
        }),
    coverageFile,
    new OpenCoverSettings()
        .ExcludeByAttribute("System.CodeDom.Compiler.GeneratedCodeAttribute")
        .WithFilter("+[MyProjectName*]*")
        .WithFilter("-[*Test*]*"));

This is just a first try, but this basic approach might have some potential to be refined and incorporated into the Tools infrastructure.

[gep13]

@jrnail23 while I see what you are doing, wouldn't another property on the Settings base class called AdditionalArguments have the same effect? The base class for the Tool could then, assuming that there is a value in the property, simply append it to the end of the generated arguments list from the other populated Settings.

So it would become:

new OpenCoverSettings()
        .ExcludeByAttribute("System.CodeDom.Compiler.GeneratedCodeAttribute")
        .WithFilter("+[MyProjectName*]*")
        .WithFilter("-[*Test*]*")
        .AdditionalArguments("-hideskipped:All"));

In general, regardless of how it it implemented, I think this is a great idea. This allows for changes to the tools, i.e. addition of some arguments to the tool, to be immediately usable within a Cake script, rather than waiting for the an update to the Tool. Great idea! 👍

[jrnail23]

@gep13, you're right, this is just what I did as a workaround in my script itself, without modifying Cake at all.

In other words, this is something you can use right now 😄

[gep13]

@jrnail23 gotcha 👍 Sounds like we are all on the same page :-D

[patriksvensson]

Yes. I agree with @gep13. Additional arguments would be really nice.