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

[Studio][Tests] Use absolute drive path for windows studio name #6645

Merged
merged 2 commits into from Jun 11, 2019

Conversation

@smacfarlane
Copy link
Contributor

commented Jun 11, 2019

When exiting the studio inside an Await session, the CWD would remain at the [Habitat] junction location. This caused the Linux style path of /hab/studios/studio-internals-test to not be resolvable and the studio rm would fail, resulting in a failing test.

This changes the studio name to use Windows-style drive letter paths, allowing the hab studio rm to behave as expected.

Signed-off-by: Scott Macfarlane smacfarlane@chef.io

Use absolute drive path for studio name
Signed-off-by: Scott Macfarlane <smacfarlane@chef.io>

@smacfarlane smacfarlane requested review from baumanj and eeyun as code owners Jun 11, 2019

@chef-expeditor

This comment has been minimized.

Copy link

commented Jun 11, 2019

Hello smacfarlane! Thanks for the pull request!

Here is what will happen next:

  1. Your PR will be reviewed by the maintainers.
  2. If everything looks good, one of them will approve it, and your PR will be merged.

Thank you for contributing!

@smacfarlane smacfarlane changed the title Use absolute drive path for studio name [Studio][Tests] Use absolute drive path for studio name Jun 11, 2019

@smacfarlane smacfarlane changed the title [Studio][Tests] Use absolute drive path for studio name [Studio][Tests] Use absolute drive path for windows studio name Jun 11, 2019

@baumanj
Copy link
Member

left a comment

This is outside my area of familiarity

@@ -8,7 +8,7 @@ $env:HAB_NOCOLORING = "true"
# Await has trouble parsing non-ascii glyphs
$env:HAB_GLYPH_STYLE = "ascii"
$exit_code = 0
$studio_name = "/hab/studios/studio-internals-test"
$studio_name = "c:\hab\studios\studio-internals-test"

This comment has been minimized.

Copy link
@mwrock

mwrock Jun 11, 2019

Contributor

This is fine our purposes here and indeed almost any scenario. I always like to use:

$ExecutionContext.SessionState.Path.GetUnresolvedProviderPathFromPSPath("/some/path/I/want/to/cannonacalize")

This will always transform the given string to a Windows friendly absolute path whether or not the path actually exists and applicable to one's current context. In otherwords, if you were in the d:\ drive, this would resolve to the d drive.

Note there is also the simpler and more intuitively named Resolve-Path which does the same thing but requires the given path to exist. If I know or expect the path to already exist, I prefer this command.

# Add the same behavior for `hab studio rm` to ensure that the studio is fully cleaned up before
# stopping the Await session.
$retry = 0
while(($retry -lt 10) -and (Test-Path "$studio_name")) {
while(($retry -lt 5) -and (Test-Path "$studio_name")) {

This comment has been minimized.

Copy link
@mwrock

mwrock Jun 11, 2019

Contributor

I still like keeping this to a higher number because if we reach the limit, subsequently deleting the studio environment is guaranteed to fail because of open file handles from the hab binary.

[PR Feedback] Increase timeout, Cannonicalize path
Signed-off-by: Scott Macfarlane <smacfarlane@chef.io>
@mwrock

mwrock approved these changes Jun 11, 2019

@smacfarlane smacfarlane merged commit f0e92a9 into master Jun 11, 2019

5 checks passed

DCO This commit has a DCO Signed-off-by
Details
buildkite/habitat-sh-habitat-master-verify Build #2272 passed (39 minutes, 2 seconds)
Details
buildkite/habitat-sh-habitat-master-website Build #2566 passed (1 minute, 34 seconds)
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
expeditor/config-validation Validated your Expeditor config file
Details

@smacfarlane smacfarlane deleted the sm/fix-windows-studio-test branch Jun 11, 2019

chef-ci added a commit that referenced this pull request Jun 11, 2019

Update CHANGELOG.md with details from pull request #6645
Obvious fix; these changes are the result of automation not creative thinking.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.