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

Correct win_path index checks to allow for 0 #50970

Merged
merged 8 commits into from
Feb 14, 2019
Merged

Correct win_path index checks to allow for 0 #50970

merged 8 commits into from
Feb 14, 2019

Conversation

jalandis
Copy link
Contributor

@jalandis jalandis commented Dec 21, 2018

What does this PR do?

Fixes an issue with win_path module that improperly handled an index of 0.

Normalizes the path to allow for the example in bug report #45622. Allows the name to end with a path separator.

Fixes a win_path state comment ensuring an index of 0 is reported properly.

What issues does this PR fix or reference?

Fixes #45622

Previous Behavior

Failed to move a path from 1,2,... to 0.

New Behavior

test path:
  win_path.exists:
    - name: 'C:\Program Files\test\'
    - index: 0
----------
          ID: test path
    Function: win_path.exists
        Name: C:\Program Files\test\
      Result: True
     Comment: Moved C:\Program Files\test from index 2 to 0.
     Started: 15:05:44.304000
    Duration: 0.0 ms
     Changes:
              ----------
              index:
                  ----------
                  new:
                      0
                  old:
                      2

Summary for local
------------
Succeeded: 1 (changed=1)
Failed:    0
------------

Tests written?

Yes. Added assertion to validate adding to path at index 0.

Commits signed with GPG?

Yes

Please review Salt's Contributing Guide for best practices.

See GitHub's page on GPG signing for more information about signing commits with GPG.

@jalandis
Copy link
Contributor Author

The state and module files were using 2 different but similar methods to sanitize the directory input. I am second guessing my decision to rely on the win_path modules normalize_dir method.

Should normalize_dir be moved to a utility module or switched to os.path.normpath?

@dwoz dwoz requested a review from twangboy February 13, 2019 22:22
@dwoz dwoz merged commit 91a551c into saltstack:develop Feb 14, 2019
garethgreenaway added a commit to garethgreenaway/salt that referenced this pull request Sep 19, 2019
dwoz added a commit that referenced this pull request Jan 6, 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.

state.win_path with index 0 add the path at the end
3 participants