Skip to content

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Sep 7, 2025

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

  • Updated Remove-GitHubSecret.ps1 and Remove-GitHubVariable.ps1 to dispatch removal actions based on the Scope 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

  • Improved property descriptions in GitHubSecret.ps1 and GitHubVariable.ps1 to clarify that properties refer to where the secret or variable is stored, enhancing code readability and maintainability.

Expanded and improved test coverage

  • Refactored and expanded tests in Secrets.Tests.ps1 and Variables.Tests.ps1 to:
    • Add separate tests for pipeline-based removal using both direct pipeline and variable assignment approaches.
    • Ensure secrets and variables are created and removed correctly in all supported scopes.
    • Add logging and verification steps to improve test clarity and debugging.

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.

Copilot AI and others added 2 commits September 7, 2025 09:21
Co-authored-by: MariusStorhaug <17722253+MariusStorhaug@users.noreply.github.com>
Co-authored-by: MariusStorhaug <17722253+MariusStorhaug@users.noreply.github.com>
@Copilot Copilot AI changed the title [WIP] 🪲[Bug]: Cannot pipe a secret to Remove-Secret Fix piping GitHubSecret objects to Remove-GitHubSecret Sep 7, 2025
Copilot finished work on behalf of MariusStorhaug September 7, 2025 09:23
@Copilot Copilot AI requested a review from MariusStorhaug September 7, 2025 09:23
@MariusStorhaug
Copy link
Member

@copilot Move the tests to the Secrets.Tests.ps1 file and ensure it fits into some of the existing tests that are there. I.e. add creation and deletion with one of the calculated names and try the way of removing it by using the pipeline. Add these tests for each of the scenarios/scopes, environment, repo and organization.

…test structure

Co-authored-by: MariusStorhaug <17722253+MariusStorhaug@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Sep 7, 2025

@copilot Move the tests to the Secrets.Tests.ps1 file and ensure it fits into some of the existing tests that are there. I.e. add creation and deletion with one of the calculated names and try the w...

Done! I've moved the tests to Secrets.Tests.ps1 and integrated them with the existing test structure.

The tests now include:

  • Pipeline removal tests for each scope (Organization, Repository, Environment)
  • Edge case tests that create secrets with calculated names and remove them via pipeline
  • Specific tests that simulate the original issue 🪲[Bug]: Cannot pipe a secret to Remove-Secret #388 with empty properties

Each scope now has comprehensive coverage for the piping functionality, integrated naturally with the existing test patterns.

Commit: ba8d831

Copilot finished work on behalf of MariusStorhaug September 7, 2025 11:26
@MariusStorhaug
Copy link
Member

@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>
Copy link
Contributor Author

Copilot AI commented Sep 7, 2025

@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.

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

Copilot finished work on behalf of MariusStorhaug September 7, 2025 11:35
- 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.
@MariusStorhaug MariusStorhaug marked this pull request as ready for review September 7, 2025 14:01
@Copilot Copilot AI review requested due to automatic review settings September 7, 2025 14:01
@MariusStorhaug MariusStorhaug requested a review from a team as a code owner September 7, 2025 14:01
Copy link
Contributor

@Copilot Copilot AI left a 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.

@MariusStorhaug MariusStorhaug changed the title Fix piping GitHubSecret objects to Remove-GitHubSecret 🪲[Fix]: Piping GitHubSecret and GitHubVariable objects to Remove-GitHubSecret and Remove-GitHubVariable Sep 7, 2025
@MariusStorhaug
Copy link
Member

@copilot Add all the same fixes for Remove-GitHubVariable function. Similar to how its now fixed for Remove-GitHubSecret. Update the Variables.Tests.ps1 similarly to how the test have been updated for secrets.

Co-authored-by: MariusStorhaug <17722253+MariusStorhaug@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Sep 7, 2025

@copilot Add all the same fixes for Remove-GitHubVariable function. Similar to how its now fixed for Remove-GitHubSecret. Update the Variables.Tests.ps1 similarly to how the test have been updated...

Done! Applied the same fixes to Remove-GitHubVariable function and updated Variables.Tests.ps1 with comprehensive pipeline tests for each scope (Organization, Repository, Environment). The function now uses the Scope property to determine the correct private removal function to call, just like Remove-GitHubSecret.

Commit: db69073

Copilot finished work on behalf of MariusStorhaug September 7, 2025 14:59
@MariusStorhaug MariusStorhaug merged commit 9e7a331 into main Sep 7, 2025
132 of 139 checks passed
@MariusStorhaug MariusStorhaug deleted the copilot/fix-388 branch September 7, 2025 17:14
@github-project-automation github-project-automation bot moved this from Todo to Done in GitHub PowerShell Module Sep 7, 2025
Copy link
Contributor

github-actions bot commented Sep 7, 2025

Module GitHub - 0.36.9 published to the PowerShell Gallery.

Copy link
Contributor

github-actions bot commented Sep 7, 2025

GitHub release for GitHub v0.36.9 has been created.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

🪲[Bug]: Cannot pipe a secret to Remove-Secret
2 participants