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

filesystem: improve Windows support #3606

Merged
merged 17 commits into from Nov 19, 2018

Conversation

Projects
None yet
4 participants
@mhackethal
Contributor

mhackethal commented Nov 13, 2018

improve filesystem.rb to support windows.
Split into 2 classes LinuxFileSystemResource / WindowsFileSystemResource
Add filesystem to verify a FS-type ( currently not for linux because missing test server )
Size on Windows is converted to GB - discussion about this welcome

Signed-off-by: markus hackethal mh@it31.de

Improve filesystem.rb to support windows
improve filesystem.rb to support windows.
Split into 2 classes LinuxFileSystemResource / WindowsFileSystemResource
Add filesystem to verify a FS-type ( currently not for linux because missing test server )
Size on Windows is converted to GB - discussion about this welcome
@jquick

This comment has been minimized.

Contributor

jquick commented Nov 13, 2018

Looking good @mhackethal. You will need to reformat this Resource according to our Rubocop rules. You can test the file via bundle exec rubocop filesystem.rb.

Update filesystem.rb
update according to rubocopy rules
@mhackethal

This comment has been minimized.

Contributor

mhackethal commented Nov 13, 2018

Thanks for the hint. I already recognized the "rubocop style".
@jquick anyway to rerun it online? I dont have the required environment to perform it.

mhackethal added some commits Nov 13, 2018

Update filesystem.rb
next update for style guide
Signed-off-by: markus hackethal <mh@it31.de>
Update filesystem.rb
Signed-off-by: markus hackethal <mh@it31.de>
Update filesystem.rb
next try
@mhackethal

This comment has been minimized.

Contributor

mhackethal commented Nov 14, 2018

@jquick Would you mind to assist me? I struggle currently with the unit testing. I adjusted the unit test for Filesystem but it didnt pass. https://travis-ci.org/inspec/inspec/jobs/454909571
Atm i am a little bit lost since i can not detect the root cause. A short hint from your side or a link to right "howto" would be very helpful so i can do it better next time.

Thanks 🥇

@jquick

Looking really nice @mhackethal!

Show resolved Hide resolved lib/resources/filesystem.rb Outdated
Show resolved Hide resolved test/unit/resources/filesystem_test.rb Outdated

mhackethal added some commits Nov 14, 2018

Update filesystem.rb
update to reflect also windows os
@jquick

This comment has been minimized.

Contributor

jquick commented Nov 14, 2018

Looks like its still having issues. Ill pull it down this afternoon and play with it a bit and see what I can find. I think we may need to mock some of the commands that are used in the testing.

@mhackethal

This comment has been minimized.

Contributor

mhackethal commented Nov 14, 2018

Yes youre right. I am currently try to understand how "mock" is working. I found the folders
but i dont understand the strucuture and how to create a output file. At least i undestand that the test is not done against a real resource. Thats why unit testing fails. Since the windows cmd is "get-wmiobject" and a file mock/cmd/get-wmiobject already exists i am now totally lost 👎

Sorry for the additional work on your side. I really gave it a try . Many thanks for your support.

mhackethal and others added some commits Nov 14, 2018

Fix the testing code for filesystem.
Signed-off-by: Jared Quick <jquick@chef.io>
@jquick

This comment has been minimized.

Contributor

jquick commented Nov 14, 2018

@mhackethal - I was able to get this working here mhackethal#1

This mocks the long windows command you run and returns the cmd file accordingly. Since the command is long I used the sha of the command instead which is supported.

@jquick

This comment has been minimized.

Contributor

jquick commented Nov 14, 2018

You can also run your unit tests like this to check it:

((HEAD detached at mhackethal/patch-1))> bundle exec m test/unit/resources/filesystem_test.rb
Run options: -n "/^(test_0001_verify\\ filesystem\\ on\\ linux|test_0002_verify\\ filesystem\\ on\\ windows|test_0001_fails\\ on\\ FreeBSD\\ \\(unix\\-like\\))$/" --seed 21363

# Running:

.Command not mocked:
    $disk = Get-WmiObject Win32_LogicalDisk -Filter "DeviceID='c:'"
    $disk.Size = $disk.Size / 1GB
    $disk | select -property DeviceID,Size,FileSystem | ConvertTo-Json
    SHA: 2df93b941844efec1eee05c311cb4b4e8e3fd8b31808b785a70bd1a46298c716
E.

Finished in 0.402526s, 7.4529 runs/s, 7.4529 assertions/s.

  1) Error:
Inspec::Resources::FileSystemResource#test_0002_verify filesystem on windows:
Inspec::Exceptions::ResourceSkipped: Unable to get available space for partition c:
    /Users/jquick/Chef/inspec/lib/resources/filesystem.rb:90:in `info'
    /Users/jquick/Chef/inspec/lib/resources/filesystem.rb:47:in `size'
    /Users/jquick/Chef/inspec/test/unit/resources/filesystem_test.rb:15:in `block (2 levels) in <top (required)>'

3 runs, 3 assertions, 0 failures, 1 errors, 0 skips

This was how I easily got the SHA for the command.

Merge pull request #1 from inspec/jq/help_mhackethal
Fix the testing code for filesystem.
@mhackethal

This comment has been minimized.

Contributor

mhackethal commented Nov 14, 2018

is it possible to get the SHA key when i edit over GitHub Website?
i was able to trace it down that i Need an Addition to /helper/resource,rb

@mhackethal

This comment has been minimized.

Contributor

mhackethal commented Nov 14, 2018

And again some Kind of failure 👎

 1) Failure:
428
PluginInstallerInstallationTests#test_install_a_gem_with_invalid_depends_from_rubygems_org [C:/projects/inspec/test/unit/plugin/v2/installer_test.rb:221]:
429
Expected # encoding: UTF-8
430
"can't activate inspec-test-fixture-0.1.2, already activated inspec-test-fixture-0.1.0" to include # encoding: UTF-8
431
"Could not find 'fake_plugin_dependency' (>= 0)".

No idea what i am doing wrong.
Maybe you can support again @jquick

@jquick

This comment has been minimized.

Contributor

jquick commented Nov 15, 2018

Hey @mhackethal , That is not related to your changes. It is an issue we see sometimes in our testing framework and I need to nail down why it's happening. I think the last thing we need for you is to sign off your commit. You can do this by:

git commit --amend -s
git push -f
@mhackethal

Signed-off-by: markus hackethal mh@it31.de

@jquick

This comment has been minimized.

Contributor

jquick commented Nov 15, 2018

Sorry @mhackethal, our policy is that it has to be part of the commit message if possible.

@mhackethal

This comment has been minimized.

Contributor

mhackethal commented Nov 15, 2018

yes i see was just test. Since i do everything over github website. I have to change a file to submit the sign-off. But i wanted to test if there is a different way.
I will setup a Dev environment next days but atm no time

Update filesystem_test.rb
Signed-off-by: Markus Hackethal <mh@it31.de>
@jquick

This comment has been minimized.

Contributor

jquick commented Nov 15, 2018

Lookin good!

@mhackethal

This comment has been minimized.

Contributor

mhackethal commented Nov 15, 2018

some off-topic question would you mind to check #3607 too?
I get travis and appveyor error which are not related to my change. But can not solve it.

@jquick

jquick approved these changes Nov 15, 2018

Thanks for all your work on this @mhackethal !

@miah

Thank you for this contribution @mhackethal! I just have one minor question about the the example in the description.

Show resolved Hide resolved lib/resources/filesystem.rb Outdated

@clintoncwolfe clintoncwolfe changed the title from Improve filesystem.rb to support windows to filesystem: improve Windows support Nov 15, 2018

mhackethal added some commits Nov 16, 2018

Update filesystem.rb
Change its 'filesystem' to 'type' according to recommendation from @miah
Signed-off-by: Markus Hackethal <mh@it31.de>
Update filesystem.rb
removed comment.
Signed-off-by: Markus Hackethal <mh@it31.de>
Update filesystem.rb
removed whitespace.
Signed-off-by: Markus Hackethal <mh@it31.de>
@miah

miah approved these changes Nov 19, 2018

Perfect. Thanks for this contribution @mhackethal !!

@jquick jquick merged commit 86cf553 into inspec:master Nov 19, 2018

4 checks passed

DCO This commit has a DCO Signed-off-by
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
expeditor/config-validation Validated your Expeditor config file
Details

@mhackethal mhackethal deleted the mhackethal:patch-1 branch Nov 27, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment