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

(#2854) Add helper to read config values #2952

Merged
merged 4 commits into from
Jun 20, 2023

Conversation

TheCakeIsNaOH
Copy link
Member

@TheCakeIsNaOH TheCakeIsNaOH commented Dec 28, 2022

Description Of Changes

This adds a PowerShell helper to get config values from the Chocolatey configuration file. This helper is intended to be used to allow hook scripts to access configuration values set in the config.

Motivation and Context

See #2854 and chocolatey-community/chocolatey-hooks#6 (comment)

Testing

  1. Create a package called getconfig with a chocolateyInstall.ps1 with these contents:
$ErrorActionPreference = 'Stop'

$val1 = Get-ChocolateyConfigValue -configKey "addedTextValue"
Write-Host "Should have 'validAddedValue' output: '$val1'"

$val2 = Get-ChocolateyConfigValue -configKey "addedNumberValue"
Write-Host "Should have '123456' output: '$val2'"

$val3 = Get-ChocolateyConfigValue -configKey "commandExecutionTimeoutSeconds"
Write-Host "Should have '2700' output: '$val3'"

$val4 = Get-ChocolateyConfigValue -configKey "cacheLocation"
Write-Host "Should have empty output:  '$val4'"

$val5 = Get-ChocolateyConfigValue -configKey "nonExistentKey"
Write-Host "Should have empty output:  '$val5'"
  1. Run .\choco.exe config set --name="addedTextValue" --value="validAddedValue"
  2. Run .\choco.exe config set --name="addedNumberValue" --value="123456"
  3. Run `.\choco.exe install getconfig --source=".\getconfig" --yes
  4. Validate that the Write-Host calls output the correct values.

Operating Systems Testing

  • Windows 10 22H2

Change Types Made

  • Bug fix (non-breaking change).
  • Feature / Enhancement (non-breaking change).
  • Breaking change (fix or feature that could cause existing functionality to change).
  • Documentation changes.
  • PowerShell code changes.

Change Checklist

  • Requires a change to the documentation.
  • Documentation has been updated.
  • Tests to cover my changes, have been added.
  • All new and existing tests passed?
  • PowerShell code changes: PowerShell v2 compatibility checked?

Related Issue

Fixes #2854

Copy link
Member

@corbob corbob left a comment

Choose a reason for hiding this comment

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

A couple of suggestions. As well, we would do well to either add some Pester tests that exercises these, or open an issue to track adding pester tests.

Some tests that I see above and beyond the testing listed in the PR description would be to change the config values, and ensure that the changed values are read. As well as adding non-standard values and ensure that they too can be read. (Edit: The first sentence was written before I had looked again at the testing instructions. That was already covered there.)

@TheCakeIsNaOH
Copy link
Member Author

As well, we would do well to either add some Pester tests that exercises these, or open an issue to track adding pester tests.

Agreed. I don't really have a good setup to iterate on the Pester tests, so a separate issue, or waiting until the behavior is nailed down is the way to go.

@pauby
Copy link
Member

pauby commented Jun 20, 2023

we would do well to either add some Pester tests that exercises these, or open an issue to track adding pester tests.

Let's get some tests added as part of this PR before we merge it. If we're adding code line this, we need to be adding tests too.

@corbob
Copy link
Member

corbob commented Jun 20, 2023

Let's get some tests added as part of this PR before we merge it. If we're adding code line this, we need to be adding tests too.

@TheCakeIsNaOH as you're not in a position to test the pester tests here, I'll get these tests added to the PR.

TheCakeIsNaOH and others added 4 commits June 20, 2023 08:10
This adds a PowerShell helper to get config values from the Chocolatey
configuration file. This helper is intended to be used to allow hook scripts
to access configuration values set in the config.
Add pester tests around the functionality added by
Get-ChocolateyConfigValue
The catch of Get-ChocolateyConfigValue should reasonably be an error and
not a warning. Changing this to send an error so the package can handle
it if desired.
Copy link
Member

@gep13 gep13 left a comment

Choose a reason for hiding this comment

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

LGTM!

@gep13 gep13 merged commit 70f8dc0 into chocolatey:develop Jun 20, 2023
@gep13
Copy link
Member

gep13 commented Jun 20, 2023

@corbob thanks for getting this over the line, and thanks to @TheCakeIsNaOH for the initial implementation!

@TheCakeIsNaOH TheCakeIsNaOH deleted the get-config-helper branch June 20, 2023 22:26
@TheCakeIsNaOH
Copy link
Member Author

Thanks @corbob for creating the tests.

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.

Create PowerShell helper function to read config values
4 participants