Skip to content
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

[master] Porting #49195 to master #54553

Closed

Conversation

garethgreenaway
Copy link
Contributor

@garethgreenaway garethgreenaway commented Sep 18, 2019

Porting #49195 to master

@garethgreenaway garethgreenaway requested a review from a team September 18, 2019 23:45
@ghost ghost requested review from waynew and removed request for a team September 18, 2019 23:45
@garethgreenaway garethgreenaway changed the title [2019.2.1] Porting #49195 to 2019.2.1 [master] Porting #49195 to master Sep 24, 2019
@garethgreenaway garethgreenaway changed the base branch from 2019.2.1 to master September 24, 2019 22:57
@mchugh19
Copy link
Contributor

This is a dependency on porting #49395 and #53068

@@ -112,55 +139,41 @@ def run_test(**kwargs):
return "Test must be a dictionary"


def run_state_tests(state):
def run_state_tests(state, saltenv=None, check_all=False):
'''
Execute all tests for a salt state and return results
Nested states will also be tested

:param str state: the name of a user defined state
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need a docstring for saltenv, too

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I think the docstring for state is now out of date.

@@ -321,6 +322,42 @@ def test__assert_less_equal3(self):
mybool = sc_instance._SaltCheck__assert_less_equal(aaa, bbb)
self.assertEqual(mybool, 'Pass')

def test__assert_empty(self):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's great that we have additional tests here.

Maybe I'm missing something, but I'm not seeing tests for the new runtests functionality (saltenv, checkall).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is part of a series of updates for the saltcheck module, the latest of which is #53068. Might it make sense to allow these ports into master, and handle the doc and test updates as part of the most recent PR?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe?

Should #53068 to point at master?

@mchugh19
Copy link
Contributor

I've ported these changes and the other neon saltcheck updates into #55613

@waynew waynew closed this Dec 26, 2019
@waynew waynew removed the master-port label Feb 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants