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

Add AppVeyor Build #365

Merged
merged 8 commits into from
Jan 11, 2017
Merged

Add AppVeyor Build #365

merged 8 commits into from
Jan 11, 2017

Conversation

rkeithhill
Copy link
Collaborator

Of course this requires someone to setup an AppVeyor account - I think.
Not sure what's up with the line endings. On my file system, this file has CRLF at the end of every line.

@dahlbyk dahlbyk force-pushed the rkeithhill/appveyor branch 2 times, most recently from b616a12 to 1b871e8 Compare January 9, 2017 03:16
I'm not 100% confident this will work the first time. I'm an AppVeyor n00b but it shouldn't take too long to whip into shape.

Also, add *.yml to .gitattributes to force LF normalization
@dahlbyk
Copy link
Owner

dahlbyk commented Jan 9, 2017

YAML is rather specific about EOL normalization, so I'm assuming what you were seeing is a quirk of the syntax highligher. I've taken the liberty of rewriting this branch with *.yml added to .gitattributes and the new file normalized to LF. Go ahead and fetch then reset your local branch to the remote (git reset "@{u}" if you set up tracking with git push -u).

AppVeyor is set up for this project and the PR is building. 🎉

# Nothing to build at this point, just run Pester tests
$testResultsFile = Join-Path $pwd TestResults.xml
$testing = @{
Script =
Copy link
Owner

Choose a reason for hiding this comment

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

Build failed because no value is specified here:

OutputFile : The term 'OutputFile' is not recognized as the name of a cmdlet, function, script file, or operable program. Check the spelling of the name, or if a path was included, verify that the path is correct and try again.

@dahlbyk dahlbyk changed the title Rkeithhill/appveyor Add AppVeyor Build Jan 9, 2017
@rkeithhill
Copy link
Collaborator Author

Thanks for the links! I wonder if the [Runspace]::DefaultRunspace.InitialSessionSta ... issue is due to v3. I thought I had tested that on v3. If it had be executed with v2 it wouldn't have executed that line.

@rkeithhill
Copy link
Collaborator Author

Hmm, the runspace line works on PS v3 in my VM. Grrr..

@rkeithhill
Copy link
Collaborator Author

OK,the test failure now is legit. I think I need to update my Get-SshPath tests to reflect changing that function back to using $env:home and Resolve-Path. The think about Resolve-Path is that it returns nothing for a path that doesn't exist. Is that the correct semantics for this function?

@rkeithhill
Copy link
Collaborator Author

Aha, looks like folks view an empty result from Get-SshPath as a bug #268. @dahlbyk Can you confirm that? If so then Resolve-Path is not the way to go. It will error and not return anything if the path doesn't exist.

@wekempf
Copy link

wekempf commented Jan 9, 2017

I've never liked Resolve-Path as it does two things you rarely want to do together (normalize the path AND validate it exists). What's not clear here to me is why you're using Resolve-Path? In fact, I'm not sure what this Get-SshPath command does. There's no help for it, and the result when I run it is the path to a key file. What's the point of that?

Anyway, if you need to normalize (make absolute) a path without validating it exists, I've got code that does that. Of course, since this will always be in the file system, it actually should be fairly trivial to implement using System.IO.Path, while my code works even if you've set-location to some other provider.

@rkeithhill
Copy link
Collaborator Author

What's not clear here to me is why you're using Resolve-Path

I was putting the code back to a state - more or less - before I changed it. However I did change it to not use ~ which isn't valid in all drives. So I think the fix here is to simply remove Resolve-Path as it no longer needs to resolve ~.

@dahlbyk
Copy link
Owner

dahlbyk commented Jan 10, 2017

I'm good with removing Resolve-Path.

This happens when the specified $File parameter doesn't exist.  This is not desired.  Update Pester tests to reflect this.
@rkeithhill
Copy link
Collaborator Author

@dahlbyk I'm happy with this. OK if I merge it?

@dahlbyk dahlbyk merged commit c25f364 into master Jan 11, 2017
@dahlbyk dahlbyk deleted the rkeithhill/appveyor branch January 11, 2017 03:36
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.

3 participants