Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/make invoke pester calls ci agnostic #23

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

leyshon
Copy link
Contributor

@leyshon leyshon commented Aug 10, 2016

In order to better allow the module to be used with other CI tools I've renamed the Invoke-PesterForTeamCity cmdlet and added a parameter to specify the CI tool. I've updated the references to this cmdlet and made sure that the existing Invoke-TeamCityCiUpload will continue to function as it did.

…rCi and have added a parameter to specify the CI tool you are using (or 'other' which is the default) to allow for different code paths for different tools. As a result the teamcity message is now only written if you specify the CiTool as 'TeamCity'
…ou can pass this through as a parameter. Default is set to 'other'
…topusScriptTestSuite to ensure the TeamCity messages are correctly written by Invoke-PesterForCi
… also added a test to ensure that the Write-TeamCity message does not take place when the CiTool parameter is not set to TeamCity
@paulmarsy
Copy link
Contributor

Nice improvement, I don't see a problem accepting this, a few considerations:

  • I think the ValidateSet on the CiTool parameter in Invoke-PesterForCi (an internal function not exposed by the module) was likely intended for Invoke-OctopusScriptTestSuite (listed in the psd1 file to be exported)?
  • The original intent of Write-TeamCityMessage which is just a thin wrapper for Write-Host is that it could be paired with a similar one to allow the kind of differentiation in output messaging that you're achieving with the parameter to prevent the ##teamcity message in Invoke-PesterForCi. What do you think of either:
    • Creating another thin wrapper such as Write-CiMessage (bad name but something non-Ci tool specific) and find+replacing the existing calls to use that, and have Write-TeamCityMessage for those specific messages containing ##teamcity only? And inside Write-TeamCityMessage check to see which CI tool is in use, if it isn't TeamCity then return without doing anything?
    • If that would harm the code readability then Write-TeamCityMessage should either be renamed or removed, as it doesn't make sense for it to be called when CiTool is not TeamCity.
  • The reason for the change (apart from the improved naming, which stands on it's own merit) is to prevent that one call in Invoke-Pester which contains ##teamcity, to meet your requirement there might be a simpler solution? I.e. using a switch rather than the validateset to just stop (or enable) the automation tool messages?

The validate set lists a number of other CI Tools, which are you using out of interest? Could the module be extended to take advantage of it, have you already got it hooked up to your CI tool?

@dragon788
Copy link

This sounds pretty slick to add as it would allow testing with AppVeyor and TeamCity/Jenkins.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants