Skip to content

Conversation

@rjmholt
Copy link
Collaborator

@rjmholt rjmholt commented Jun 22, 2018

PR Summary

Testing to validate #7126. (This should be merged first)

Adds testing specifically for all the code around ModuleSpecification objects. In particular:

  • The ToString() funtionality (paired with parsing back to the object)
  • The equality comparisons
  • Hashcode logic

PR Checklist

@rjmholt rjmholt added WG-Engine core PowerShell engine, interpreter, and runtime WG-Cmdlets-Core cmdlets in the Microsoft.PowerShell.Core module and removed WG-Engine core PowerShell engine, interpreter, and runtime labels Jun 22, 2018
@rjmholt rjmholt mentioned this pull request Jun 22, 2018
11 tasks
# Take a base hashtable and produce all possible
# combinations of that hashtable with the keys
# and values in the key/value hashtable
function PermuteHashtable
Copy link
Collaborator

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?

Copy link
Collaborator Author

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()
}
Copy link
Collaborator

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()

Copy link
Collaborator Author

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")
Copy link
Collaborator

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.

@iSazonov
Copy link
Collaborator

@adityapatwardhan Seems the PR is ready to merge.

@adityapatwardhan adityapatwardhan merged commit dfa1a31 into PowerShell:master Jun 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

WG-Cmdlets-Core cmdlets in the Microsoft.PowerShell.Core module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants