-
Notifications
You must be signed in to change notification settings - Fork 87
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
More cleanup of test output and stability fixes #464
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Ryan Davis <zenspider@chef.io>
This fixes using `m` and other tooling due to bad requires / non-standard pathing. Signed-off-by: Ryan Davis <zenspider@chef.io>
Signed-off-by: Ryan Davis <zenspider@chef.io>
There's legit problems being reported that we can't see because of poor signal:noise. Signed-off-by: Ryan Davis <zenspider@chef.io>
Signed-off-by: Ryan Davis <zenspider@chef.io>
Made the defaults into procs so they could probe the environment at the time the test is run. This allows the require to be at the top of the file to allow tests to be run in isolation instead of expecting a fixed order. Signed-off-by: Ryan Davis <zenspider@chef.io>
The constants are for restoring the globals only. Use the globals. Signed-off-by: Ryan Davis <zenspider@chef.io>
I don't think the code should be writing to stdout/stderr at all, it should use the logger... but in the meantime, capture and quiet the output. Signed-off-by: Ryan Davis <zenspider@chef.io>
Esp not at the method level. There doesn't seem to be a reason for it, so I just made them local vars. Signed-off-by: Ryan Davis <zenspider@chef.io>
…ated. Signed-off-by: Ryan Davis <zenspider@chef.io>
Signed-off-by: Ryan Davis <zenspider@chef.io>
It's unstable and breaking other tests, but that's next... Signed-off-by: Ryan Davis <zenspider@chef.io>
For some reason `plat.stubs(:cisco_ios?).returns(true)` winds up creating the method and bombing out every ssh-oriented test that follows. By creating the method and defaulting to false, all methods are fine after the stub is cleared. Basically, you shouldn't stub a method that doesn't exist... and when you get down to it, you shouldn't design these methods that change their behavior based on these sorts of internals. Use a factory method instead and grab a proper subclass based on what you want to do. But actually running the tests is a good start... so... yeah. Signed-off-by: Ryan Davis <zenspider@chef.io>
Code Climate has analyzed commit 8abde51 and detected 1 issue on this pull request. Here's the issue category breakdown:
View more on Code Climate. |
miah
approved these changes
May 29, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I've found some tests that fail in isolation but pass in combination, which is unfortunately hard to isolate... That'll have to come later.