New execution module to make testing state/highstate logic easy #33642
Conversation
| ''' | ||
|
|
||
| def __init__(self): | ||
| self.__opts__ = salt.config.minion_config('/etc/salt/minion') |
There was a problem hiding this comment.
This won't work across different platforms, i.e. Windows. We need to find another way to get opts in here.
There was a problem hiding this comment.
Thanks Mike. I appreciate the info. I'm open to solutions.....perhaps I
should detect the OS, and pull accordingly?
On Wed, Jun 1, 2016 at 12:02 PM, Mike Place notifications@github.com
wrote:
In salt/modules/salt_check.py
#33642 (comment):
- Author: William Cannon william period cannon at gmail dot com'''
+import os
+import os.path
+import yaml
+import salt.client
+import salt.minion
+import salt.config
+class SaltCheck(object):
- '''
- This class implements the salt_check
- '''
- def init(self):
self.**opts** = salt.config.minion_config('/etc/salt/minion')This won't work across different platforms, i.e. Windows. We need to find
another way to get opts in here.—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/saltstack/salt/pull/33642/files/3c72408d16a62590dc78015f835e5b7d06b86d59#r65401400,
or mute the thread
https://github.com/notifications/unsubscribe/AA3k_VKaESgdvGH5p17382rJVqOhaKUwks5qHbspgaJpZM4IqzaF
.
There was a problem hiding this comment.
Is __opts__ not in there already? We should be able to just use __opts__
There was a problem hiding this comment.
I'd rather figure out a way to see if we can get the loader to inject opts in here, but that's a little outside the scope of what you're trying to do, obviously. There might be a way to pull it out of the module namespace here where the loader should be putting it but I'd need to poke around a bit to see for sure.
There was a problem hiding this comment.
@thatch45 Probably not in an instance method like this. Though, that might have worked at one time. I forget exactly.
There was a problem hiding this comment.
thats odd, maybe I am remembering something incorrectly, lemme take a look
There was a problem hiding this comment.
when I test it both situations work for me:
class Bar(object):
def __init__(self):
self.opts = __opts__
def run():
bar = Bar()
return bar.opts
There was a problem hiding this comment.
Agreed - definitely works. It was only my unit tests failing. But the code itself works nicely. Any advice on using opts in unit tests?
There was a problem hiding this comment.
Ahh, makes sense.
@cachedout you have been here more recently than I, do you remember how to get opts in unit tests?
There was a problem hiding this comment.
The short answer is that..you don't. Right now, unit tests are designed to operate with them. If you need, them, create an opts dictionary by hand containing the keys that you need or use the salt.config.minion_config call that you had earlier to generate them. I recommend the first approach.
|
I really like this |
|
@thatch45 I agree. This feels like a very solid approach. @wcannon I'd like to get some of the implementation details cleaned up as we work through the review and some docs added. I saw your blog post, so that would be a good start if we could get those details added to the docs if you're willing to. |
|
Thanks for the feedback. Glad you like the approach. Happy to add docs and etc, and to figure out how to pull in opts on other operating systems. Also, might need some guidance on adding to docs and etc. This is my first time contributing to salt code. :-) |
|
Adding docs here is easy, just add them to the module level docstring at the top. |
|
Sorry, I hit the wrong button.... |
|
I believe the code is ready for another round of tests. Do I need to issue a new pull request to get the latest changes? |
|
@wcannon No need. The test suite will re-run automatically after you pushed your changes. Should be done in about an hour. |
|
@wcannon Could you please address the lint errors outlined here? https://jenkins.saltstack.com/job/PR/job/salt-pr-lint-n/2272/violations/ Don't worry about the servicenow errors. Those are from somewhere else. |
cachedout
added
the
Pending-Discussion
|
Believe it or not I just realized that I need to convert my standard python unit tests to the salt unit tests and set up a new dev environment for running the tests. Needless to say - I'll be working on this later tonight and tomorrow night. |
|
@wcannon You're awesome! Thank you! |
|
Go Go Jenkins! |
|
Hi @wcannon I'd really like to get this in this week. Is there a way I can help? |
|
Hi @cachedout Short answer - maybe. I ran out of time to work on this for a couple of days - work/personal life stuff. I just fixed my documentation, which helped. Right now I have a problem with opts working in the module, but not in my unit test. Not sure why though. I'm going to try to get this sorted out tomorrow night. If not, I'll ping you for help. :-) |
|
Hi @cachedout ok, I could use a little guidance. I'm getting an error that is puzzling. I set up my dev environment using the docs - with virtualenv. Started the master and minion using my non-root account. salt-call test.ping works fine. Calling my module fails in a big strange way: (salt-check) wcannon@salt-check-pull-request:~/salt/salt/modules$ salt saltdev salt_check.run_test test='{"module_and_function": "test.echo","assertion": "assertEqual","expected-return": "This works!","args":["This works!"] }' |
|
No apologies necessary. I figured you were super busy. Thanks for the info on the tornado ioloop issue. Interestingly, the issue only happens with the testing environment (unit test). The module works fine outside of the unit tests. :-) |
|
@wcannon Can you please rebase this PR against develop and see if the situation with the tests improves? |
|
Mike - I'll try to get some time to do it. But, for the next two weeks I'm On Mon, Jul 18, 2016 at 9:01 AM, Mike Place notifications@github.com
|
|
Hi @wcannon Just checking in with you on this one. Could you let me know where we are? Thanks. :] |
|
Hi Mike, You probably heard that I had an immediate family member with a severe The next few weeks are likely to be somewhat difficult and busy. However, ttys On Sat, Aug 6, 2016 at 5:59 PM, Mike Place notifications@github.com wrote:
|
|
I'm so sorry to hear it. All of our best to you and your family. |
|
Hi @wcannon Just checking in with you here. Did you want to keep working on this or should we close it for the time being? |
|
Hi Mike, The answer is yes. I'm going to attempt to write the tests this week. On Wed, Sep 28, 2016 at 9:29 PM, Mike Place notifications@github.com
|
|
Hi @wcannon Just checking in here. Thanks! |
|
Hi Mike, Just started setting up my dev environment yesterday - long hiatus. If I On Thu, Oct 20, 2016 at 1:20 AM, Mike Place notifications@github.com
|
|
Guys - I spent a couple days attempting to get the salt testing environment Please feel free to integrate my salt-check code module.
On Thu, Oct 20, 2016 at 9:11 AM, William Cannon william.cannon@gmail.com
|
|
Go Go Jenkins! |
2 similar comments
|
Go Go Jenkins! |
|
Go Go Jenkins! |
|
We just haven't had activity on this for a while so we need to close it. The code itself will still be in the branch if somebody wants to pick this up and run with it. |
What does this PR do?
Adds one remote execution module
What issues does this PR fix or reference?
unknown
Tests written?
Yes - not sure where to put them