-
Notifications
You must be signed in to change notification settings - Fork 4
🪲[Fix]: Piping GitHubSecret
and GitHubVariable
objects to Remove-GitHubSecret
and Remove-GitHubVariable
#499
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
Conversation
Co-authored-by: MariusStorhaug <17722253+MariusStorhaug@users.noreply.github.com>
Co-authored-by: MariusStorhaug <17722253+MariusStorhaug@users.noreply.github.com>
@copilot Move the tests to the |
…test structure Co-authored-by: MariusStorhaug <17722253+MariusStorhaug@users.noreply.github.com>
Done! I've moved the tests to The tests now include:
Each scope now has comprehensive coverage for the piping functionality, integrated naturally with the existing test patterns. Commit: ba8d831 |
@copilot Remove the part of the test where we set some values to null or blank. The objects themselves will have these as empty, so just getting them and piping them should be enough. |
As requested, removed the parts where we set Repository and Environment properties to empty strings in the edge case tests. The GitHubSecret objects retrieved from the API naturally have these properties empty based on their scope, making the tests more realistic and simpler. Co-authored-by: MariusStorhaug <17722253+MariusStorhaug@users.noreply.github.com>
Done! Removed the explicit property assignments where we were setting Repository and Environment properties to empty strings. The tests now rely on the natural state of GitHubSecret objects as returned by the API, making them more realistic and simpler. Commit: a44e9ce |
- Created TEMPLATE.ps1 for structuring tests with Pester. - Implemented Teams.Tests.ps1 to validate GitHub Teams API functionalities, including team creation, retrieval, updating, and deletion. - Developed Users.Tests.ps1 to test user-related API calls, such as user retrieval and updates, including email management. - Added Variables.Tests.ps1 to cover GitHub variable management, including setting, updating, and removing variables for users and organizations. - Ensured all tests utilize appropriate logging and context management for better traceability during execution.
- Created TEMPLATE.ps1 for structuring Pester tests with authentication cases. - Added Teams.Tests.ps1 to test GitHub Teams API functionalities, including team creation, retrieval, updating, and deletion. - Introduced Users.Tests.ps1 to validate user-related API calls, such as fetching user details and updating user information. - Implemented Variables.Tests.ps1 to test GitHub repository and organization variable management, including setting, updating, and removing variables.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request fixes a bug in the Remove-GitHubSecret
function where piping GitHubSecret objects with empty or whitespace properties would fail to properly determine the scope for secret removal. The fix implements proper scope detection logic based on non-empty property values.
- Implements robust scope detection logic that checks for non-empty/whitespace Owner, Repository, and Environment properties
- Routes secret removal to the appropriate scope-specific function (environment, repository, or organization)
- Adds comprehensive test coverage for edge cases with empty and whitespace properties
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
File | Description |
---|---|
src/functions/public/Secrets/Remove-GitHubSecret.ps1 | Replaces generic parameter passing with explicit scope detection and routing to appropriate removal functions |
tests/Secrets.Tests.ps1 | Renames existing tests for clarity and adds new edge case tests for empty/whitespace property handling |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
…ve-GitHubSecret function
GitHubSecret
and GitHubVariable
objects to Remove-GitHubSecret
and Remove-GitHubVariable
@copilot Add all the same fixes for |
Co-authored-by: MariusStorhaug <17722253+MariusStorhaug@users.noreply.github.com>
Done! Applied the same fixes to Commit: db69073 |
Module GitHub - 0.36.9 published to the PowerShell Gallery. |
GitHub release for GitHub v0.36.9 has been created. |
This pull request improves the handling and testing of secret and variable removal in the GitHub module. The main focus is on making the removal functions (
Remove-GitHubSecret
,Remove-GitHubVariable
) more robust and scope-aware, and on expanding the test coverage to ensure correct behavior across different usage patterns and scopes.Key changes include:
Functional improvements to removal logic
Remove-GitHubSecret.ps1
andRemove-GitHubVariable.ps1
to dispatch removal actions based on theScope
property of each item, ensuring the correct removal function is called for environment, repository, or organization scopes, and providing clear error handling for unsupported scopes.Documentation and class property clarifications
GitHubSecret.ps1
andGitHubVariable.ps1
to clarify that properties refer to where the secret or variable is stored, enhancing code readability and maintainability.Expanded and improved test coverage
Secrets.Tests.ps1
andVariables.Tests.ps1
to:These changes make the module's behavior more predictable and easier to test, especially when handling secrets and variables in different scopes and using pipeline operations.