Skip to content
This repository has been archived by the owner on Feb 19, 2019. It is now read-only.

add label to new file types in the Install-ChocolateyFileAssociation command and enhancements to Start-ChocolateyProcessAsAdmin #564

Merged
merged 1 commit into from Sep 18, 2014

Conversation

mwrock
Copy link
Contributor

@mwrock mwrock commented Sep 2, 2014

This includes a fix to the Install-ChocolateyFileAssociation command to prevent the windows explorer New Item context menu from having items removed when this command replaces file extensions with new types.

More significantly this makes Start-ChocolateyProcessAsAdmin easier to work with by:

  1. Encoding the command string to reduce the coding friction around nested quotes and multi line powershell blocks. As can be seen from Install-ChocolateyFileAssociation, I can now construct my elevated script using an easier to read and write here string.
  2. Do a better job of capturing errors from the elevated script and bubbling them back up to the package error. While testing with the Install-ChocolateyFileAssociation command, My script raised an error that I would see appear and then disappear. The final error and what was in the log simply tells me there is an error. This dumps the standard error stream of the elevated process to a file. If the process was a powershell process the stream is output as CLIXML and this will deserialize that to its human readable form.
  3. This removes duplicate errors. I still noticed a duplicate error after these changes but there were far fewer than before.

I realize this change is fairly significant and if others dont see the value add here, I'm happy to resubmit with just the File Association specific changes.

…command and enhancements to Start-ChocolateyProcessAsAdmin
@ferventcoder
Copy link
Contributor

Don't worry about the appveyor failure. It's going to fail for awhile methinks...

@ferventcoder
Copy link
Contributor

There are some tests that seem to fail in that environment. I need to get to the bottom of those.

@mwrock
Copy link
Contributor Author

mwrock commented Sep 2, 2014

I'll take a look at tests tonight. I really should have run those locally.

@ferventcoder
Copy link
Contributor

https://ci.appveyor.com/project/ferventcoder/chocolatey/build/1.0.5 - would be neat to see the test output show up but it is not adding the test output file for some reason.

@gep13
Copy link
Member

gep13 commented Sep 2, 2014

Looks like for Pester, you have to tell it where the test results end up:

http://www.appveyor.com/docs/running-tests

For other systems it discovers them automatically.

@ferventcoder
Copy link
Contributor

@gep13 I thought I could just use artifacts but apparently not...

@mwrock
Copy link
Contributor Author

mwrock commented Sep 3, 2014

Local tests are passing as long as I am using pester v2.0.2. I small just pushed a change to the build scripts that fix an issue if like me one has a version of pester greater than 2.0.2. However I doubt this is why appveyor is failing.

@mwrock
Copy link
Contributor Author

mwrock commented Sep 3, 2014

all the appveyor failing tests are related to Get-CheckSumValid. Based on the errors it seems like CheckSum.exe is not where Chocolatey expects it to be. Is there a way in Appveyor to browse the chocolatey files that are installed?

@ferventcoder
Copy link
Contributor

@mwrock Right. That's what I was trying to say before. The failures are not related to your pull request, so feel free to ignore those.

@JVimes
Copy link

JVimes commented Sep 18, 2014

If the errors are unrelated, is this ready for merge, then?

ferventcoder added a commit that referenced this pull request Sep 18, 2014
add label to new file types in the Install-ChocolateyFileAssociation command and enhancements to Start-ChocolateyProcessAsAdmin
@ferventcoder ferventcoder merged commit aa85464 into chocolatey-archive:master Sep 18, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants