Skip to content

In case of failure rerun command in specified location#287

Merged
devblackops merged 5 commits intopsake:masterfrom
tiksn:fix/exec-popd-on-fail
Sep 21, 2019
Merged

In case of failure rerun command in specified location#287
devblackops merged 5 commits intopsake:masterfrom
tiksn:fix/exec-popd-on-fail

Conversation

@tiksn
Copy link
Copy Markdown
Contributor

@tiksn tiksn commented Sep 11, 2019

In case of command failure retry running command on he location passed by caller

Description

Lets say caller wan't to Exec deletion of all files inside D:\ folder and retry it max 10 times.
We are running script in E:\ folder
Old/current code would have changed location to E:\ (correctly), try and fail on deleting all files and will pop location back to E:
Next 9 attempts will run inside E:
Now we will push and pop on each attempt

Related Issue

Mistake was in #200

Motivation and Context

Mentioned in description

How Has This Been Tested?

Because this is an easy fix I just tested in small sample build script

Screenshots (if appropriate):

Types of changes

  • [ x] Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • [ x] My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • [ x] All new and existing tests passed.

@UberDoodles
Copy link
Copy Markdown
Contributor

I was confused by your description at first, but looking at the commit, I think I understand what you mean. I think your example should read :

Lets say caller wan't to Exec deletion of all files inside D:\ folder and retry it max 10 times.
We are running script in E:\ folder
Old/current code would have changed location to *D:* (correctly), try and fail on deleting all files and will pop location back to E:
Next 9 attempts will run inside E:

Is that correct?

@tiksn
Copy link
Copy Markdown
Contributor Author

tiksn commented Sep 12, 2019

I was confused by your description at first, but looking at the commit, I think I understand what you mean. I think your example should read :

Lets say caller wan't to Exec deletion of all files inside D:\ folder and retry it max 10 times.
We are running script in E:\ folder
Old/current code would have changed location to _D:_ (correctly), try and fail on deleting all files and will pop location back to E:
Next 9 attempts will run inside E:

Is that correct?

Yes, sorry for typo and confusion.

@UberDoodles
Copy link
Copy Markdown
Contributor

I've raised a pull request against your branch which adds a Pester test for your change.

@devblackops devblackops self-requested a review September 13, 2019 04:27
@devblackops
Copy link
Copy Markdown
Member

Thanks @tiksn for the fix and thanks @UberDoodles for the tests! 👍

@tiksn Once you merge the test PR in, I'll merge this.

Copy link
Copy Markdown
Member

@devblackops devblackops left a comment

Choose a reason for hiding this comment

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

Thanks @tiksn. If you can merge the tests from @UberDoodles's PR, I'll merge this. 👍

Pester spec to verify that Exec executes script block in the specified working directory
@UberDoodles
Copy link
Copy Markdown
Contributor

Sorry @tiksn; looks like my test doesn't work on Ubuntu or Mac. I'll try and fix it later and raise another pull request.

@UberDoodles
Copy link
Copy Markdown
Contributor

Sorry for the delay guys; been struggling for time.

I think I've fixed this in my branch now, but it's just based on guessing what the issue is. I don't currently have a Linux / Mac to test it on. I'll get Ubuntu running as soon as I get chance and test it out.

Altering Pester spec for Execs working directory to be cross-platform
@tiksn
Copy link
Copy Markdown
Contributor Author

tiksn commented Sep 20, 2019

Thanks @UberDoodles now all checks pass.

@devblackops devblackops merged commit 163c260 into psake:master Sep 21, 2019
@tiksn tiksn deleted the fix/exec-popd-on-fail branch September 30, 2019 07:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants