cake
cake copied to clipboard
TFBuild.IsRunningOnVSTS is confusing when using own build agent
What You Are Seeing?
I have a VSTS build agent, which is triggering a cake build on GitHub commits.
I tried using the TFBuild.IsRunningOnVSTS to check if this was a build running on VSTS, as I am also using AppVeyor.
What is Expected?
I would have expected TFBuild.IsRunningOnVSTS to return true. It returned false.
This seems to be per design as seen in the TFBuildProvider
What version of Cake are you using?
Latest
I discussed this briefly with @patriksvensson on Gitter and we kind of agreed that the IsRunningOnVSTS and IsRunningOnTFS properties could be a bit misleading.
@patriksvensson suggested a possible rename:
IsRunningOnVSTS -> IsRunningOnSharedAgent
IsRunningOnTFS -> IsRunningOnOwnAgent
To me this would be much more clear as VSTS can both run shared and own agents.
@agc93 you seem to be the original author of this feature, what do you think about this?
I don't think we should rename them, but add something similar to what is suggested to reduce confusion. I'm very open for suggestions for naming of this 😄
Okay so jumping in on this there's a difference between VSTS/TFS and hosted agent/own agent, since it's (theoretically) possible to have any combination of those.
So, short version is that, yeah this behaviour is probably not quite right since VSTS + self-hosted agent is a perfectly valid combo. You can always check whether you're on a hosted agent using the TFBuild.Environment.Agent.IsHosted property. For the system, my thinking is to probably add an additional IsRunningOnTFBuild to encompass both TFS and VSTS (and any agent type).
Would that suit your use case @Cheesebaron ? Thoughts @patriksvensson ?
EDIT: I'll add a note to the documentation for this too, since I think it probably doesn't warrant a breaking change, personally.
Any thoughts/feedback before I start on this @Cheesebaron or @patriksvensson ? I've talked to Microsoft about it, so I'll try and improve the logic as well as adding that new property.
I don't really care myself whether this is a breaking change or not. However, for people using this, they probably want their scripts to continue to work.
IsHosted is more clear.
If there is not actual distinction between IsRunningOnVSTS and IsRunningOnTFS other than one is hosted and the other is not, I would try to remove that confusion entirely.
Okay so I don't think removing IsRunningOnVSTS/IsRunningOnTFS is either a) the right call or b) something we can change at this point (that's one hell of a breaking change).
As discussed, you can already use TFBuild.Environment.Agent.IsHosted to confirm if you're on a hosted agent, regardless of the IsRunningOn* properties.
I will look into improving the logic of IsRunningOn and PR it when I get a chance.