-
Notifications
You must be signed in to change notification settings - Fork 683
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
Suppress warnings in unit test output #3828
Conversation
This does the following: - Captures warning for lack of `--sudo` with `--sudo-password` - Captures warnings for transformation of URL target in url fetcher - Changes deprecated `supports:` syntax to use new syntax Signed-off-by: Jerry Aldrich <jerryaldrichiii@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not seeing any warnings when run under m
. Could you include how you are running the tests, and what output you are seeing?
@@ -255,8 +255,13 @@ | |||
end | |||
|
|||
it 'assumes `--sudo` if `--sudo-password` is used without it' do | |||
cfg = Inspec::Config.new('sudo_password' => 'somepass') | |||
cfg.key?('sudo').must_equal true | |||
@mock_logger = Minitest::Mock.new |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What warning is being suppressed here? I don't see a warning when run on master@
[cwolfe@lodi inspec-review]$ git log --oneline -n1
8c37a614 (HEAD -> master, tag: v3.6.12, origin/master, origin/HEAD) Bump version to 3.6.12 by Chef Expeditor
[cwolfe@lodi inspec-review]$ be m test/unit/config_test.rb
Run options: -n "/^(test_0001_should\\ initialize\\ properly|test_0001_should\\ initialize\\ properly|test_0001_should\\ read\\ the\\ file\\ successfully|test_0001_should\\ read\\ the\\ file\\ successfully|test_0001_should\\ read\\ the\\ file\\ successfully|test_0001_should\\ throw\\ an\\ exception|test_0001_should\\ throw\\ an\\ exception|test_0001_should\\ throw\\ an\\ exception|test_0001_should\\ have\\ the\\ correct\\ defaults|test_0001_should\\ have\\ the\\ correct\\ defaults|test_0001_should\\ transparently\\ round\\-trip\\ the\\ options|test_0001_should\\ read\\ the\\ options|test_0001_should\\ read\\ the\\ options|test_0001_parse\\ cli\\ reporters|test_0001_parses\\ cli\\ report\\ and\\ attaches\\ target_id|test_0001_valid\\ reporter|test_0002_invalid\\ reporter\\ type|test_0003_two\\ reporters\\ outputting\\ to\\ stdout|test_0003_calls\\ `Compliance::API\\.login`\\ if\\ `opts\\[:compliance\\]\\ is\\ passed`|test_0001_raises\\ if\\ `\\-\\-password/\\-\\-sudo\\-password`\\ are\\ used\\ without\\ value|test_0002_assumes\\ `\\-\\-sudo`\\ if\\ `\\-\\-sudo\\-password`\\ is\\ used\\ without\\ it|test_0001_should\\ pass\\ the\\ credentials\\ as\\-is|test_0001_should\\ read\\ the\\ backend\\ and\\ strip\\ prefixes|test_0001_should\\ unpack\\ the\\ options\\ using\\ the\\ URI\\ parser|test_0001_should\\ assign\\ the\\ options\\ correctly|test_0001_the\\ config\\ file\\ setting\\ should\\ prevail|test_0001_the\\ CLI\\ option\\ should\\ prevail|test_0001_the\\ CLI\\ option\\ should\\ prevail|test_0001_the\\ config\\ file\\ setting\\ should\\ prevail)$/" --seed 17558
# Running:
.............................
Finished in 0.131891s, 219.8785 runs/s, 712.7097 assertions/s.
29 runs, 94 assertions, 0 failures, 0 errors, 0 skips
[cwolfe@lodi inspec-review]$
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, get this. I couldn't get them to output via m
any more. If you run bundle exec rake test
though you'll see:
https://github.com/inspec/inspec/blob/master/lib/inspec/config.rb#L367
@@ -20,6 +20,17 @@ | |||
m | |||
} | |||
|
|||
def expect_url_transform |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here, when run under m
I don't see any warnings
Yup, can do. @clintoncwolfe. I also couldn't get the output to show up in |
Here is the output of
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, while I don't see the same warnings (which is problematic on my end), the changes you have here are healthy - thanks!
As this is test code, we generally only need one approver - moving ahead with this one. |
@clintoncwolfe @robbkidd helped me out a bit. It looks like running I was certain this used to not do that though. |
This does the following:
--sudo
with--sudo-password
supports:
syntax to use new syntax