Use xcpretty by default in Xcode task

[madhurig]

The detailed xcodebuild log will be uploaded along with the other logs

[madhurig]

New major versions should generally start in "preview" for a sprint

[brcrista]

When we throw we should use Error and setResult with err.message below. When JavaScript built-ins throw, they will throw Error.

[madhurig]

Good point, for some reason all places in this file didn't use it. I will update.

[madhurig]

Actually in the async/await pattern used in the tests, it does not matter. If we update the throw to use Error, we have to update setResult to use err.message. So we will have to update everywhere or keep what we have.

[brcrista]

Ok, that's fine. Error will get toString'ed to show its message anyway.

[bryanmacfarlane]

I think it should throw an Error. As @brcrista noted, anyone console.log(err) will get toString'd I believe

[brcrista]

I think this function's name is a little confusing, since we are piping both to secondTool and some file created from logPrefix. I think this is because we are trying to do two things at once: create the unique log file name and pipe output. If something is too hard to name, I find it's usually a smell that it should be more modular.

So I think this is the function we want to export:

/** Find an unused file name of the form $(Agent.TempDirectory) + logPrefix + <number> + .log. */
export function createUniqueLogFileName(logPrefix: string): string {
    let filePath: string = tl.resolve(tl.getVariable('Agent.TempDirectory'), logPrefix + '.log');
    let index = 1;
    while (tl.exist(filePath)) {
        filePath = tl.resolve(tl.getVariable('Agent.TempDirectory'), logPrefix + index.toString() + '.log');
        index++;
    }
}

Then the call site looks like:

const logFile: string = utils.createUniqueLogFileName('xcodebuild_archive');
xcodeArchive.pipeExecOutputToTool(xcPrettyTool, logFile);
utils.setTaskState('XCODEBUILD_ARCHIVE_LOG', logFile);

It's an extra LOC but I find it easier to understand the logic.

[brcrista]

You just log want to log err here. Existing code that is not throwing Error will still work, and Error will get converted to string.

[madhurig]

It changed the error message (added Error: )but looks good otherwise. I updated the impacted test as well.