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

Fix integration.modules.test_state for Windows #45308

Merged
merged 4 commits into from Jan 10, 2018

Conversation

twangboy
Copy link
Contributor

@twangboy twangboy commented Jan 6, 2018

What does this PR do?

Fixes failing tests on Windows

  • Fixes Windows-specific issues in test_state
  • Fixes an issue with the prereq_simple2 state file that had some
    strange characters that were causing failure in windows (<C2><A0>)
  • Adds a jinja to watch_any to handle Windows

Fixes an issue in file.managed where a command execution error was
being raised if the target file did not exist.

@rallytime Need to backport this to 2017.7. Should I do a separate PR?

What issues does this PR fix or reference?

saltstack/salt-ci-images#439

Tests written?

Yes

Commits signed with GPG?

Yes

deny_perms=win_deny_perms,
inheritance=win_inheritance,
reset=win_perms_reset)
try:
Copy link
Contributor

Choose a reason for hiding this comment

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

This section of code would be cleaner if you encompassed just the file.check_perms command in the try/except instead of the if is_windows check as well.

@rallytime rallytime added the bugfix-bckport will be be back-ported to an older release branch by creating a PR against that branch label Jan 9, 2018
@rallytime
Copy link
Contributor

@twangboy I can do the backport once this is ready.

This is causing a test failure, however: https://jenkins.saltstack.com/job/PR/job/salt-pr-linode-ubuntu14-n/18415/

Can you take a look?

Fixes the following tests:
- test_get_file_from_env_in_top_match
- test_issue_1896_file_append_source
- test_state_sls_id_test

Fixes in issue in file.managed where a command execution error was being
raised if the target file did not exist.
@twangboy twangboy changed the title Fix integration.modules.test_state for Windows Fix unit.states.test_file for Windows Jan 9, 2018
@twangboy twangboy changed the title Fix unit.states.test_file for Windows Fix integration.modules.test_state for Windows Jan 9, 2018
- Fix problem with invalid characters in requisites.prereq_simple2
- Fix problem with true/false commands in Windows. Need to use exit
- Fix some issues with hard-coded paths to /tmp
@twangboy
Copy link
Contributor Author

@rallytime This should fix the failing linux test.

Copy link
Contributor

@rallytime rallytime left a comment

Choose a reason for hiding this comment

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

Well done

@rallytime rallytime merged commit 32806d0 into saltstack:oxygen Jan 10, 2018
@rallytime
Copy link
Contributor

@twangboy This isn't back-porting cleanly. Can you make a separate PR for this against 2107.7?

@rallytime rallytime removed the bugfix-bckport will be be back-ported to an older release branch by creating a PR against that branch label Jan 10, 2018
@twangboy twangboy deleted the win_fix_test_state branch January 10, 2018 16:44
twangboy added a commit to twangboy/salt that referenced this pull request Jan 10, 2018
rallytime pushed a commit that referenced this pull request Jan 11, 2018
rallytime pushed a commit to rallytime/salt that referenced this pull request Jan 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants