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

Use all-in-one ms_dotnet cookbook #61

Merged
merged 6 commits into from Dec 11, 2015

Conversation

Annih
Copy link
Contributor

@Annih Annih commented Nov 26, 2015

Goal of this Pull Request

The main goal of this pull requests is to use ms_dotnet cookbook, instead of ms_dotnet2 + ms_dotnet4 + ms_dotnet45 - some powershell recipes were even implementing there own way to enable .NET.

ms_dotnet is a 'all-in-one' cookbook allowing you to configure almost all .NET versions; it has been written following the same model as this powershell cookbook and is actively maintained by @criteo.
Here are the reasons why I think powershell should use ms_dotnet:

  1. having a single dependency to setup .NET is better than multiple ones.
  2. ms_dotnet is more up-to-date than others cookbooks, it'll even soon handle .NET 4.6
  3. ms_dotnet is rspec testable on linux because it uses windows' ohais instead of Win32 API

Content of this Pull Request

Migration from ms_dotnetX to ms_dotnet

The first commit of this PR changed all references to ms_dotnet2, ms_dotnet4 and ms_dotnet5 into ms_dotnet reference.
Because a single recipe ms_dotnet::ms_dotnet4 takes care of .NET 4 and .4.5, if attributes are not set properly powershell4 recipe which require .NET 4.5 will throw an exception.
I also removed some lines of code reimplementing .NET feature activation.

Test fixes

Because migrating from one dependency cookbooks to another can be tricky, migrating from 3 to one require to be careful. One the most important thing I tried to keep in mind is to not break anything.
But ... it's hard when the tests are not passing to know if you are not breaking things; that's why I also focused on repairing the specs.

powershell recipes specs

I updated powershell recipes specs to ensure that they are properly testing the new code, and that all tests are useful - e.g. windows 7 Core does not exist, no need to test this case.
I tried to factorize at much as possible the tests, in order to avoid code/mistake duplication.

winrm and dsc recipes specs

I didn't spend too much time on theese specs, because I don't really understand why powershell cookbook include that kind of recipe; but I updated the specs to comply with other changes.

powershell module provider specs

These tests were failing for many reasons, one the worst one is that they were actually modifying the file system, I tried to remove all those file system operations by mocking them.
I also removed the use of environment variable.

Rubocop and foodcritic fix

I finally fixed few rubocop and foodcritic violations, ignoring FC009 and refactoring winrm recipe's if statements.

Fixes #54 #56, and superseeds #55 #58

cc: @aboten

ms_dotnet is a 'all-in-one' cookbook allowing you to configure almost
all .NET versions; It has been written following the same model as this
powershell cookbook and is actively maintained by @criteo.

Reason for this migration:
* Having a single dependency to setup .NET is better than multiple ones.
* ms_dotnet is more up-to-date than others cookbooks
* ms_dotnet is going to support .NET 4.6 soon
* ms_dotnet is rspec testable on linux using ohais instead of Win32 API

Other changes:
* Because a single recipe takes care of .NET 4 & 4.5, if attributes
are not set properly powershell4 recipe which require .NET 4.5 will
throw an exception.
Some lines of code reimplementing .NET activation have been removed.
* Ignore foodcritic rule sous-chefs#9 for windows package success_codes
* Refactor winrm recipe's if statements to remove one nested block
* Fix `NoMethodError: undefined method `empty' for nil:NilClass`
when powershell.winrm.thumbprint is nil
* Update powershell's recipes' specs to test the new code.
* Removed useless tests - e.g. windows 7 Core does not exist.
* Tests factorization to avoid code/mistake duplication.
Update winrm's specs to be able to test on linux/travis:
* Remove references to `chef/winr32/version`
* Mock shellout commands
Update dsc's specs to comply with powershell::powershell4 changes.
Mock properly shellout commands.
Clean up powershell_module provider specs:
* Mock file system operations and remove all actual IO from the tests!
* Remove use of environment variable

Theses tests were relying on the test machine file system:
 * It was expecting the test machine to be windows with powershell etc.
 * It was also creating "temporary" files and folders.
@Annih Annih changed the title [WIP] Use all-in-one ms_dotnet cookbook Use all-in-one ms_dotnet cookbook Nov 30, 2015
@smurawski
Copy link
Contributor

Thanks @Annih! I'm getting this pulled in and should have it merged early next week.

@smurawski smurawski merged commit 7927717 into sous-chefs:master Dec 11, 2015
smurawski added a commit that referenced this pull request Dec 11, 2015
@Annih Annih deleted the ms_dotnet_dependency branch February 20, 2021 16:47
@Annih Annih restored the ms_dotnet_dependency branch February 20, 2021 16:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Travis build fails on rspec, what now?
3 participants