-
Notifications
You must be signed in to change notification settings - Fork 8.1k
Add tests for module specifications #7140
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
Add tests for module specifications #7140
Conversation
| # Take a base hashtable and produce all possible | ||
| # combinations of that hashtable with the keys | ||
| # and values in the key/value hashtable | ||
| function PermuteHashtable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can the test structures be initialized statically? It seems to be more simple and visual than to understand this helper code. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've been thinking about this. You're definitely right that the number of possible ModuleSpecification field combinations is lower than I imagined initially.
On the other hand, for every field that could be added to a ModuleSpecification, we would have to generate exponentially more tests vs add a single field in here. Perhaps that's unlikely. But I can see something like PSEdition being added, which would take the number of static cases from 6 to 12.
| foreach ($ht in $Hashtable) | ||
| { | ||
| $ht.Clone() | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just as an aside doing @{} + $ht is essentially equivalent to .Clone()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aha! Which do you consider nicer (for our purposes)?
|
|
||
| BeforeAll { | ||
| $modSpecAsm = [Microsoft.PowerShell.Commands.ModuleSpecification].Assembly | ||
| $modSpecComparerType = $modSpecAsm.GetType("Microsoft.PowerShell.Commands.ModuleSpecificationComparer") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could use using namespace to get rid of the long type names.
|
@adityapatwardhan Seems the PR is ready to merge. |
PR Summary
Testing to validate #7126. (This should be merged first)
Adds testing specifically for all the code around ModuleSpecification objects. In particular:
ToString()funtionality (paired with parsing back to the object)PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright headerWIP:to the beginning of the title and remove the prefix when the PR is ready.[feature]if the change is significant or affects feature tests