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

CI Treat Errors As Warnings - ref #2295 #2296

Merged
merged 9 commits into from
Apr 25, 2023

Conversation

erikgraa
Copy link
Contributor

@erikgraa erikgraa commented Feb 7, 2023

PR Summary

Added parameter 'CITreatErrorsAsWarnings' to output configuration. When true, errors will be logged in CI as warnings.

Fix #2295

PR Checklist

  • PR has meaningful title
  • Summary describes changes
  • PR is ready to be merged
    • If not, use the arrow next to Create Pull Request to mark it as a draft. PR can be marked Ready for review when it's ready.
  • Tests are added/update (if required)
  • Documentation is updated/added (if required)

@fflaten
Copy link
Collaborator

fflaten commented Feb 10, 2023

Thanks for looking into this! Will give it a review the next days hopefully.

Should the option maybe be a StringOption? Ex. CILogLevel with a default of Error? Just to avoid a breaking change if we'd want to support ex. informational/notice at one point.

Thoughts on this feature @nohwnd and @ArmaanMcleod? See discussion in related issue

…evel to account for future compatibility/demands.
@erikgraa
Copy link
Contributor Author

Thanks for looking into this! Will give it a review the next days hopefully.

Should the option maybe be a StringOption? Ex. CILogLevel with a default of Error? Just to avoid a breaking change if we'd want to support ex. informational/notice at one point.

Thoughts on this feature @nohwnd and @ArmaanMcleod? See discussion in related issue

CILogLevel is a great idea! I pushed an update that reflects this.

Copy link
Collaborator

@fflaten fflaten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! :) Just need to include configuration validation and expand tests.

We should throw a more friendly error when a invalid value is used. Add a check like this just below it. 🙂

if ($PesterPreference.Output.CIFormat.Value -notin 'None', 'Auto', 'AzureDevops', 'GithubActions') {
throw "Unsupported CI format '$($PesterPreference.Output.CIFormat.Value)'"
}

tst/Pester.RSpec.Configuration.ts.ps1 Outdated Show resolved Hide resolved
Copy link
Collaborator

@fflaten fflaten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 🎉

@erikgraa
Copy link
Contributor Author

Seems this PR didn't make it to the 5.4.1 release; any chance you will get a chance to look at it in the not-so-distant future @nohwnd / @ArmaanMcleod? 🙂

@nohwnd nohwnd merged commit 06ab106 into pester:main Apr 25, 2023
@nohwnd
Copy link
Member

nohwnd commented Apr 25, 2023

Merged, thank you all. Will start preparing next minor release.

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