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 XUnit test for HandleDoubleAndSingleQuote in CompletionHelpers class #25181

Merged

Conversation

ArmaanMcleod
Copy link
Contributor

@ArmaanMcleod ArmaanMcleod commented Mar 15, 2025

PR Summary

So to make sure the HandleDoubleAndSingleQuote method in CompletionHelpers class is covered from regression, I have added tests to ensure code is covered by regressions.

PR Context

PR Checklist

Copy link
Collaborator

@iSazonov iSazonov left a comment

Choose a reason for hiding this comment

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

I suggest to add only TestHandleDoubleAndSingleQuote tests in the PR. I have many questions for rest.

@iSazonov iSazonov added CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log CL-Test Indicates that a PR should be marked as a test change in the Change Log labels Mar 15, 2025
@iSazonov iSazonov self-assigned this Mar 15, 2025
@ArmaanMcleod ArmaanMcleod changed the title Add XUnit test for all methods in CompletionHelpers class Add XUnit test for HandleDoubleAndSingleQuote in CompletionHelpers class Mar 15, 2025
@ArmaanMcleod
Copy link
Contributor Author

I suggest to add only TestHandleDoubleAndSingleQuote tests in the PR. I have many questions for rest.

All good @iSazonov. I have only included test for HandleDoubleAndSingleQuote. I think out of all methods this one is important to have tests because the logic itself is quite complicated and probably very easy to introduce regression 😄.

@ArmaanMcleod ArmaanMcleod requested a review from iSazonov March 15, 2025 12:47
@iSazonov iSazonov removed the CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log label Mar 15, 2025
@iSazonov

This comment was marked as outdated.

This comment was marked as outdated.

@iSazonov

This comment was marked as outdated.

@iSazonov
Copy link
Collaborator

I suggest to move tests for QuoteCompletionText and GetMatchingResults to other PRs. If we add QuoteCompletionText the tests should be in that PR.

@iSazonov
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@iSazonov
Copy link
Collaborator

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@iSazonov iSazonov enabled auto-merge (squash) March 16, 2025 08:35
@ArmaanMcleod
Copy link
Contributor Author

@iSazonov Looks like unrelated packaging failures for Windows CI. Might need a rerun. Not sure though if this can auto merge with PSResourceGet and CodeFactor issues.

@iSazonov iSazonov merged commit e8bf560 into PowerShell:master Mar 17, 2025
37 of 40 checks passed
Copy link
Contributor

microsoft-github-policy-service bot commented Mar 17, 2025

📣 Hey @ArmaanMcleod, how did we do? We would love to hear your feedback with the link below! 🗣️

🔗 https://aka.ms/PSRepoFeedback

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CL-Test Indicates that a PR should be marked as a test change in the Change Log
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants