vmware-archive / powernsx

PowerShell module that abstracts the VMware NSX-v API to a set of easily used PowerShell functions
173 stars 89 forks source link

Framework for Pester tests #36

Closed devblackops closed 7 years ago

devblackops commented 8 years ago

First off, excellent work on this module. Clearly a ton of work has been put into it.

With a module this size and potential user base, I think a proper Pester test framework is needed in order to ensure quality. With that, I'd like to propose a few changes to the repository layout and a set of psake tasks and Pester tests to validate things like function help is complete and accurate. This could be later extended to include unit tests for every function and integration tests for common workflows.

Proposed structure

I created some psake tasks for various test scenarios. These include running the module through the PowerShell Script Analyzer, testing function help is complete and accurate, and the manifest is correct.

You can check out my fork here which has the changes I'm suggesting: https://github.com/devblackops/powernsx/tree/module-tests

This model works well to ensure the code quality stays high. If you decide to integrate this project with something like AppVeyor then these tests can be executed on every code commit. In my fork there is an example appveyor.yml file that you could use to do that.

If you run .\build.ps1 -Task test you will see a bunch of Pester tests be executed. 99% of these are testing module help. When I ran it on my fork, there were 976 test failures. Nearly all of these had to do with the comment-based help missing some piece of information about the function.

If you agree with these changes, I'll submit a PR for them. If not, you can just tell me to stay in my lane 😄

pandom commented 8 years ago

Nice stuff there man. The structure of it looks good.

There is a heap of Pester tests being build out already (not committed) for other things. I suspect these can be released.

I like changes - Nick is the only repo admin and on leave until October :)

nmbradford commented 8 years ago

Yup - thanks @devblackops. This is something I've been giving some thought to lately, and I agree, it is sorely needed. What I was lacking is the experience to implement it appropriately, so thank you for the thought you've put in already! Feel free to raise a PR - ill take a look asap - prepare for lots of silly questions though!

nmbradford commented 8 years ago

Hey @devblackops, Im keen to start work on integrating this and have reviewed your proposed structure and I like it! One immediate question I had is why the move of the module and manifest to a subfolder?
Without this change we can pretty easily integrate immediately without breaking anything and start working through the resolution of individual tests. My expectation at the moment is that exported functions should have at least description and at least 1 example. Im well aware of missing param help text and its an ongoing task to keep adding to these when I can (any assistance is GREATLY appreciated here ;) )

What Im really interested in though, is adopting more structured unit testing for individual functions.

First step here is to move the current (dodgy as) integration tests to a pester framework (Im already working on this)

Then I want to start working on implementing unit tests with proper mocks so that some function testing can be done without access to an actual NSX implementation. This is going to be a lot more work and Im going to need some help! Any interest? :)

If you are ok to move back to the module/manifest in the root dir, Im happy to merge a PR with all the other modifications asap. If you can rebase your fork and submit a PR that would be awesome!

Let me know if you want to discuss any further...

devblackops commented 7 years ago

Hey @nmbradford I like to put the actual PowerShell module in a subfolder as it separates it from the scaffolding files of the project (README.md, psake tasks, GitHub templates, etc). IMHO, that keeps things cleaner but it's up to you if you want to go that route. I'm flexible. See this discussion on the Plaster project. You can also take a look at my Watchmen project to see how I use that structure.

I'm not really in a position to do much with NSX at the moment so can't really offer much in the way of testing other than my opinion 😄.

With a project of this type that is essentially a wrapper against a remote API, without some type of test API instance to use, mocking the API is the only way to go and that could get unwieldy very fast with all the possible calls that are made. I'm not sure on the best way to go in that regard. I have a similar problem with my Citrix NetScaler module. It just wraps the NetScaler REST API with PowerShell-friendly commands. I don't really want to mock all the possible calls to the API so don't have proper unit tests setup.

nmbradford commented 7 years ago

Thanks @devblackops, I reached the same conclusion pretty quickly, but I really appreciate your comment though as I was worried I was just being dense :). For now, I'm focusing on migrating the existing tests to pester tests and wrapping them in a test module that allows for generic testing as much as possible. I've made good progress on this and am hoping to merge some changes this week that at least make the tests executable by anyone that has a (disposable!) NSX environment available. I also want to incorporate the module and meta tests you recommended pretty much as is, so watch this space...

nmbradford commented 7 years ago

Added basic Pester test frame work in PR #81. Work on support for PowerShell Core meant I really needed to improve the testing area. Certainly more work to be done, but I think the foundation is laid now - thanks for the suggestions @devblackops. Take a look and let me know if you have any comments.