cake

cake copied to clipboard

Right now, Cake simply shells out to the the underlying appveyor.exe. As a result, silent failures in things like UploadTestResults are not caught.

We should correctly handle appveyor.exe as a Tool, so that exit codes are correctly handled.

This was discussed here: https://github.com/cake-build/cake/issues/1097#issuecomment-234791625

@agc93 just checking, will you be able to look at this in a near future or should we bump it to next release?

Probably bump, sorry 😢 I'm looking at a couple of the VS issues at the moment.

@agc93 no worries, bumped it to v0.19.0

Hoping for an opinion from @patriksvensson / @devlead / @gep13 here :smile:

Where do we stand on changes to the IAppVeyorProvider interface? I was planning on moving the UploadArtifactSettings and UploadArtifactType (possibly also the MessageCategoryType and TestResultsType) into a new Cake.Common.Tools.AppVeyor namespace since, to my mind, they're inputs to the tool.

Problem being, they're also in the IAppVeyorProvider interface, so if I do, two things happen:

1. We couple the provider to the tool. At the moment, it sort of is anyway, on account of the shelling out this PR is supposed to fix
2. We change a public interface definition. I think in this instance this seems like not too much of a problem.

Thoughts on approach here? Just trying to avoid duplicating these settings and types..

@agc93 can we keep compatibility with existing scripts?

@cake-build/cake-team going to bump this one again...

@cake-build/cake-team going to bump this one.

@agc93 Bumping this one given we're working towards Cake v2.0 release so any breaking changes needed to IAppVeyorProvider might be easier to squeeze in now