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

State mock #30118

Merged
merged 5 commits into from Jan 6, 2016

Conversation

Projects
None yet
6 participants
@thatch45
Member

thatch45 commented Jan 1, 2016

This adds the "mock" option to state.sls and state.highstate. This allows for the state to be run without calling anything. The returned data is the return as if the states all ran successfully.

What would be nice to have here would be the ability to make mock states so that you could program what the mocked returns would look like.

@thatch45

This comment has been minimized.

Show comment
Hide comment
@thatch45

thatch45 Jan 1, 2016

Member

@jfindlay @whiteinge let me know if this meets our needs, at the least this is the hard part so you guys should be able to easily take it from here

Member

thatch45 commented Jan 1, 2016

@jfindlay @whiteinge let me know if this meets our needs, at the least this is the hard part so you guys should be able to easily take it from here

@thatch45

This comment has been minimized.

Show comment
Hide comment
@thatch45

thatch45 Jan 1, 2016

Member

BTW, this does also report argument warnings

Member

thatch45 commented Jan 1, 2016

BTW, this does also report argument warnings

@whiteinge

This comment has been minimized.

Show comment
Hide comment
@whiteinge

whiteinge Jan 1, 2016

Contributor

35 LOC. Game changing addition. This checks out as a bona fide Tom pull req.

Contributor

whiteinge commented Jan 1, 2016

35 LOC. Game changing addition. This checks out as a bona fide Tom pull req.

@s0undt3ch

This comment has been minimized.

Show comment
Hide comment
@s0undt3ch

s0undt3ch Jan 1, 2016

Member

Happy New Year!!!!

Member

s0undt3ch commented Jan 1, 2016

Happy New Year!!!!

name = cdata['args'][0]
else:
name = cdata['kwargs']['name']
return {'name': name,

This comment has been minimized.

@s0undt3ch

s0undt3ch Jan 1, 2016

Member

We should probably pull the warnings key, if present, from cdata.

@s0undt3ch

s0undt3ch Jan 1, 2016

Member

We should probably pull the warnings key, if present, from cdata.

This comment has been minimized.

@thatch45

thatch45 Jan 1, 2016

Member

No need,it is added by the state system to the ret.

@thatch45

thatch45 Jan 1, 2016

Member

No need,it is added by the state system to the ret.

This comment has been minimized.

@s0undt3ch

s0undt3ch Jan 1, 2016

Member

Awesome!

@s0undt3ch

s0undt3ch Jan 1, 2016

Member

Awesome!

@s0undt3ch

This comment has been minimized.

Show comment
Hide comment
@s0undt3ch

s0undt3ch Jan 1, 2016

Member

Perhaps the mock states, considering <parent dir>/<a dir>/a_state.sls, should probably live under <parent dir>/<a dir>/mocks...

We could probably allow specifying the mock state to use to test different scenarios.

The mock state should have state ids with the same name as the state id being mocked.

... Just suggestions...

Member

s0undt3ch commented Jan 1, 2016

Perhaps the mock states, considering <parent dir>/<a dir>/a_state.sls, should probably live under <parent dir>/<a dir>/mocks...

We could probably allow specifying the mock state to use to test different scenarios.

The mock state should have state ids with the same name as the state id being mocked.

... Just suggestions...

@s0undt3ch

This comment has been minimized.

Show comment
Hide comment
@s0undt3ch

s0undt3ch Jan 1, 2016

Member

Or the mocks loader...

Member

s0undt3ch commented Jan 1, 2016

Or the mocks loader...

@s0undt3ch

This comment has been minimized.

Show comment
Hide comment
@s0undt3ch

s0undt3ch Jan 1, 2016

Member

The unitbtest failures seem valid. The call mocks need to be updated....

Member

s0undt3ch commented Jan 1, 2016

The unitbtest failures seem valid. The call mocks need to be updated....

Show outdated Hide outdated salt/state.py Outdated
@cachedout

This comment has been minimized.

Show comment
Hide comment
@cachedout

cachedout Jan 1, 2016

Contributor

Yup, this is good. Very good. Very, very good.

Contributor

cachedout commented Jan 1, 2016

Yup, this is good. Very good. Very, very good.

@cachedout cachedout added the Awesome label Jan 1, 2016

@cachedout

This comment has been minimized.

Show comment
Hide comment
@cachedout

cachedout Jan 4, 2016

Contributor

Weird. It looks like two copies of the test suite started at the same time. Re-testing.

Contributor

cachedout commented Jan 4, 2016

Weird. It looks like two copies of the test suite started at the same time. Re-testing.

@cachedout

This comment has been minimized.

Show comment
Hide comment
@cachedout

cachedout Jan 4, 2016

Contributor

Go Go Jenkins!

Contributor

cachedout commented Jan 4, 2016

Go Go Jenkins!

@thatch45

This comment has been minimized.

Show comment
Hide comment
@thatch45

thatch45 Jan 5, 2016

Member

This error has me confused, it makes it look like the pr code is not being called. Maybe it is name overloading with the test suite

Member

thatch45 commented Jan 5, 2016

This error has me confused, it makes it look like the pr code is not being called. Maybe it is name overloading with the test suite

@cachedout

This comment has been minimized.

Show comment
Hide comment
@cachedout

cachedout Jan 5, 2016

Contributor

It's in the test suite. I'll take care of it today and get this merged. No worries.

Contributor

cachedout commented Jan 5, 2016

It's in the test suite. I'll take care of it today and get this merged. No worries.

@thatch45

This comment has been minimized.

Show comment
Hide comment
@thatch45

thatch45 Jan 5, 2016

Member

oh, I changed the internal options and kept the interface at "mock", so if tests pass from my change we should be good. If not, thanks for hunting it down!

Member

thatch45 commented Jan 5, 2016

oh, I changed the internal options and kept the interface at "mock", so if tests pass from my change we should be good. If not, thanks for hunting it down!

@cachedout

This comment has been minimized.

Show comment
Hide comment
@cachedout

cachedout Jan 5, 2016

Contributor

Sure thing. Let's see what the tests say and I'll shepard this one on through. Thanks Tom!

Contributor

cachedout commented Jan 5, 2016

Sure thing. Let's see what the tests say and I'll shepard this one on through. Thanks Tom!

@thatch45

This comment has been minimized.

Show comment
Hide comment
@thatch45

thatch45 Jan 5, 2016

Member

Thank you for putting up with my PRs, I wish I had more time to work on them!

Member

thatch45 commented Jan 5, 2016

Thank you for putting up with my PRs, I wish I had more time to work on them!

cachedout added a commit to cachedout/salt that referenced this pull request Jan 6, 2016

@cachedout cachedout merged commit 5c7853a into saltstack:develop Jan 6, 2016

2 of 5 checks passed

default Merged build finished.
Details
jenkins/salt-pr-linode-ubuntu14.04-n Salt PR - Linode Ubuntu 14.04 #3915 — FAILURE
Details
jenkins/salt-pr-rs-cent7-n Salt PR - RS CentOS 7 #11084 — FAILURE
Details
jenkins/salt-pr-clone Salt PR - Clone Repository #12497 — SUCCESS
Details
jenkins/salt-pr-lint-n Salt PR - Code Lint #12196 — SUCCESS
Details

cachedout added a commit that referenced this pull request Jan 6, 2016

@meggiebot

This comment has been minimized.

Show comment
Hide comment
@meggiebot

meggiebot Jan 6, 2016

Need in 2015.8

meggiebot commented Jan 6, 2016

Need in 2015.8

@rallytime rallytime referenced this pull request Jan 6, 2016

Merged

Fix #30118 #30185

rallytime added a commit to rallytime/salt that referenced this pull request Jan 6, 2016

@rallytime

This comment has been minimized.

Show comment
Hide comment
@rallytime

rallytime Jan 6, 2016

Contributor

@meggiebot #30185 was backported to include the fix to the test suite from @cachedout. Please see #30189.

Contributor

rallytime commented Jan 6, 2016

@meggiebot #30185 was backported to include the fix to the test suite from @cachedout. Please see #30189.

@meggiebot

This comment has been minimized.

Show comment
Hide comment
@meggiebot

meggiebot Jan 6, 2016

Got it, thanks.

meggiebot commented Jan 6, 2016

Got it, thanks.

cro added a commit to cro/salt that referenced this pull request Jan 12, 2016

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