Reject configuration values we cannot use, warn about keys we do not know - #2982
Open
nohwnd wants to merge 1 commit into
Open
Reject configuration values we cannot use, warn about keys we do not know#2982nohwnd wants to merge 1 commit into
nohwnd wants to merge 1 commit into
Conversation
…know A configuration hashtable ignored anything it did not recognize and never said a word, so a misspelled option left the run on the default with nothing to notice, and so did a correct key holding a value of the wrong type. The helpers read a failed cast as "not specified", which also hits people who spelled everything right, because values from a JSON or psd1 file arrive as strings. A value the option cannot use now throws while the configuration is built, naming the option, what it expects and what it got. That is never intentional, so there is nothing to lose by being strict. An unknown key is collected instead, and Invoke-Pester warns about all of them at once. A hashtable may carry keys meant for something else, so throwing there could break a working setup. A key that is present but null keeps meaning "not set" (#2219). Fix #2975 🤖
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix #2975
Option 3 from the issue, which @fflaten picked.
A value the option cannot use now throws while the configuration is built:
Nobody writes a value the option cannot read on purpose, so there is nothing to lose by being strict. It also covers the case where the key is spelled right, which is how values coming out of a JSON or psd1 file behave, they arrive as strings.
An unknown key is collected rather than thrown on, because a hashtable may carry keys meant for something else, and
Invoke-Pesterwarns about all of them at once:A key that is present but null still means "not set", so an unset variable does not throw (#2219).
How it works
PesterConfigurationcollects the keys that match no section and no option, and exposes them withGetUnknownKeys(). A method and not a property, so it stays out of the console output of the configuration object.Mergecarries them across, becauseInvoke-Pestermerges onto the default before it reports anything.The warning is emitted once, in
Invoke-Pester. That is the funnel for every path (-Configuration @{ }, a configuration built byNew-PesterConfiguration -Hashtable, and$PesterPreference = @{ }in a test file), so nothing is missed at run time and nobody gets the same warning twice. A bare[PesterConfiguration]@{ }outside a run stays silent,GetUnknownKeys()is there if you want to look.Known option names come from reflection over the section's
Optionproperties, so nothing has to be kept in sync by hand.A key counts as known only when looking it up by the option's own name finds it. Comparing case-insensitively on its own is not enough, a dictionary with a case-sensitive comparer holds
runwithout answering toRun, so the value would never be read and the key really is unknown.Verification
test.ps1passes, 2906 Pester tests and the P tests, no existing test relied on the silent behavior.Invoke-ScriptAnalyzeronsrc/Main.ps1reports the same 9 pre-existing findings asmain. 15 tests added totst/PesterConfiguration.Tests.ps1covering each rejected shape, null keeping the default, an int still being accepted for a decimal option, case-insensitive matching, the merge, and the three warning cases.Docs are not updated yet, tell me if you want a page about this on pester.dev and I will send it to the docs repo.
🤖