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

Why no MoRefs in the TestHelper VMware.VimAutomation.Core? #74

Closed
lucdekens opened this issue Jan 27, 2019 · 8 comments
Closed

Why no MoRefs in the TestHelper VMware.VimAutomation.Core? #74

lucdekens opened this issue Jan 27, 2019 · 8 comments
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@lucdekens
Copy link
Contributor

In the testhelper module VMware.VimAutomation.Core properties that should be of type ManagedObjectReference, are defined as the actual object.
For example:

public class HostConfigManager
{
public HostDateTimeSystem DateTimeSystem { get; set; }
public ManagedObjectReference NetworkSystem { get; set; }
public HostServiceSystem ServiceSystem { get; set; }
}

Where I would expect

public class HostConfigManager
{
public ManagedObjectReference DateTimeSystem { get; set; }
public ManagedObjectReference NetworkSystem { get; set; }
public ManagedObjectReference ServiceSystem { get; set; }
}

with a new object
public class ManagedObjectReference : System.IEquatable
{
public string Type { get; set; }
public string Value { get; set; }
...

This has impact on how the Mocks are set up.

Am I missing something?

@lucdekens lucdekens changed the title Why no MoRef in the TestHelper VMware.VimAutomation.Core Why no MoRefs in the TestHelper VMware.VimAutomation.Core? Jan 27, 2019
@SimeonGerginov
Copy link
Contributor

Yes, for the example you have provided:

 public class HostConfigManager
 {
     public HostDateTimeSystem DateTimeSystem { get; set; }
     public ManagedObjectReference NetworkSystem { get; set; }
     public HostServiceSystem ServiceSystem { get; set; }
 }

The three types can easily be replaced with the new type you proposed. The problem I see is that the new type should have the following structure:

 public class ManagedObjectReference : System.IEquatable
 {
     public string Type { get; set; }
     public string Value { get; set; }

     public void UpdateDateTimeConfig(HostDateTimeConfig config)
     {
     }
     
     public void UpdateDnsConfig(HostDnsConfig dnsConfig)
     {
     }

     public void UpdateServicePolicy(string serviceId, string servicePolicyValue)
     {
     }
 }

So potentially this class will become very large, so we think the best solution is to have separate classes.

@lucdekens
Copy link
Contributor Author

Why would for example the UpdateDateTimeConfig method, need to be defined on the ManagedObjectReference class?
I would expect it as a method on the HostDateTimeSystem class, just like in the vSphere API.
That would also make the mapping of the TestHelpers to the vSphere API more realistic.

@SimeonGerginov
Copy link
Contributor

Sorry, I think I have misunderstood your previous example. With the Type property you can specify the type for example HostDateTimeSystem and with the Value property you will pass the value of the object ?

@lucdekens
Copy link
Contributor Author

No, the Type and Value would behave as a regular MoRef.
You use the complete object as a value to the Get-View -Id cmdlet.
That call returns the actual object.

The reason for suggesting this, I can see code that will use this Get-View -Id format to retrieve vSphere objects.
If we implement this in the TestHelper module, I think we can actually cover that part of the code in our tests.
As an example, assume we have a Get-View -Id in a Try-Catch construct.
If the Get-View behaves as a regular Get-View when passed a MoRef, we can for example also test the Catch part (without the need to start using Throws in our mocks).

@SimeonGerginov
Copy link
Contributor

Yes this will be a good solution. Do you want to work on it ?

@lucdekens
Copy link
Contributor Author

Sure, I'll combine that with #75

@SimeonGerginov
Copy link
Contributor

Ok, I am assigning you to the issue.

@SimeonGerginov SimeonGerginov added the enhancement New feature or request label Jan 29, 2019
SimeonGerginov pushed a commit that referenced this issue Jan 30, 2019
…nction in the TestHelpers module VMware.VimAutomation.Core now supports all parametersets

* Fixes for issue #74 and #75

 #74 Added class definition for ManagedObjectReference
     HostConfigManager now contains ManagedObjectReference entries

 #75 The Get-View function in the TestHelpers module VMware.VimAutomation.Core now supports all parametersets

Signed-off-by: Luc Dekens <dekens.luc@gmail.com>

* Small fixes on formatting

Signed-off-by: SimeonGerginov <simeongerginov1@gmail.com>
@SimeonGerginov
Copy link
Contributor

Added to the repository via PR #82. Closing the issue.

SimeonGerginov added a commit to SimeonGerginov/dscr-for-vmware that referenced this issue Jan 31, 2019
… build (#1)

* Added class definition for ManagedObjectReference and The Get-View function in the TestHelpers module VMware.VimAutomation.Core now supports all parametersets

* Fixes for issue vmware#74 and vmware#75

 vmware#74 Added class definition for ManagedObjectReference
     HostConfigManager now contains ManagedObjectReference entries

 vmware#75 The Get-View function in the TestHelpers module VMware.VimAutomation.Core now supports all parametersets

Signed-off-by: Luc Dekens <dekens.luc@gmail.com>

* Small fixes on formatting

Signed-off-by: SimeonGerginov <simeongerginov1@gmail.com>

* Travis update: (Build 76)

[skip ci]

Signed-off-by: Travis CI <travis@travis-ci.org>
@SimeonGerginov SimeonGerginov added this to the 2.0-Consider milestone Apr 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants