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

Fix test=True for file.directory with recurse ignore_files/ignore_dirs. #29708

Merged
merged 2 commits into from Dec 15, 2015

Conversation

Projects
None yet
2 participants
@lagesag
Contributor

lagesag commented Dec 15, 2015

Fixes #25723.

Cause: _check_directory() failed to implement "ingore_files" and
"ignore_dirs" as valid options in "recurse" for directory() state.

Changes:

  • New _RECURSE_TYPE const lists possible values for "recurse" list.
  • New _get_recurse_set() function validates "recurse". Raises TypeError
    or ValueError and returns a set of strings.
  • directory() (test=False) uses _get_recurse_set().
  • _check_directory() (test=True) uses _get_recurse_set().
  • _check_directory() takes "ignore_files" and "ignore_dirs" into account
    when recursively investigating a directory.

Additional change: _check_directory() always checks the root directory
(specified in "name"). Previously it was skipped when "recurse" was enabled.

Fix test=True for file.directory with recurse ignore_files/ignore_dirs.
Fixes #25723.

Cause: _check_directory() failed to implement "ingore_files" and
"ignore_dirs" as valid options in "recurse" for directory() state.

Changes:

* New _RECURSE_TYPE const lists possible values for "recurse" list.
* New _get_recurse_set() function validates "recurse". Raises TypeError
  or ValueError and returns a set of strings.
* directory() (test=False) uses _get_recurse_set().
* _check_directory() (test=True) uses _get_recurse_set().
* _check_directory() takes "ignore_files" and "ignore_dirs" into account
  when recursively investigating a directory.

Additional change: _check_directory() always checks the root directory
(specified in "name"). Previously it was skipped when "recurse" was enabled.
Show outdated Hide outdated salt/states/file.py Outdated
Show outdated Hide outdated salt/states/file.py Outdated
@cachedout

This comment has been minimized.

Show comment
Hide comment
@cachedout

cachedout Dec 15, 2015

Contributor

HI @lagesag

Thanks for this. This will be a great fix to have in for the next release.

The logic looks pretty good here. There are some minor style issues that we would like you to clean up before we merge this. Please have a look.

Contributor

cachedout commented Dec 15, 2015

HI @lagesag

Thanks for this. This will be a great fix to have in for the next release.

The logic looks pretty good here. There are some minor style issues that we would like you to clean up before we merge this. Please have a look.

@lagesag

This comment has been minimized.

Show comment
Hide comment
@lagesag

lagesag Dec 15, 2015

Contributor

Hi @cachedout thanks for feedback. It's my first contribution. I hope it's going to be fine now. I assume the failures from the other two builds were unrelated.

Contributor

lagesag commented Dec 15, 2015

Hi @cachedout thanks for feedback. It's my first contribution. I hope it's going to be fine now. I assume the failures from the other two builds were unrelated.

@cachedout

This comment has been minimized.

Show comment
Hide comment
@cachedout

cachedout Dec 15, 2015

Contributor

@lagesag Yes, there are some known failures right now in the test suite that can safely ignored. Thanks for contributing to Salt!

Contributor

cachedout commented Dec 15, 2015

@lagesag Yes, there are some known failures right now in the test suite that can safely ignored. Thanks for contributing to Salt!

cachedout added a commit that referenced this pull request Dec 15, 2015

Merge pull request #29708 from lagesag/fix-file-directory-test-mode
Fix test=True for file.directory with recurse ignore_files/ignore_dirs.

@cachedout cachedout merged commit aba82ab into saltstack:2015.8 Dec 15, 2015

2 of 5 checks passed

default Merged build finished.
Details
jenkins/salt-pr-linode-ubuntu14.04-n Salt PR - Linode Ubuntu 14.04 #3099 — FAILURE
Details
jenkins/salt-pr-rs-cent7-n Salt PR - RS CentOS 7 #10606 — FAILURE
Details
jenkins/salt-pr-clone Salt PR - Clone Repository #12035 — SUCCESS
Details
jenkins/salt-pr-lint-n Salt PR - Code Lint #11737 — SUCCESS
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment