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

feat(vscode): Refactored Unit Test Commands for Improved Maintainability and Reusability #6440

Merged
merged 10 commits into from
Jan 27, 2025

Conversation

samikay101
Copy link
Contributor

Requirement Checklist

work Item Link

  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes or features)

Type of Change

  • Bug fix
  • Feature
  • Other (Code Refactoring)

Current Behavior

The previous implementation duplicated logic across two commands:

  • saveBlankUnitTest
  • createUnitTest

This duplication made the code harder to maintain, with redundant calls to shared utilities like:

  • getWorkflowNode
  • promptForUnitTestName
  • processAndWriteMockableOperations

New Behavior

  • Consolidated shared logic between saveBlankUnitTest and createUnitTest to improve maintainability.
  • Introduced helper methods for:
    • Parsing and transforming unitTestDefinition outputs (parseUnitTestOutputs).
    • Processing mockable operations (processAndWriteMockableOperations).
    • Generating C# class definitions dynamically (generateCSharpClasses).
  • Updated the structure to use consistent workflows and error handling via callWithTelemetryAndErrorHandling.

Highlights:

  • Improved maintainability and reduced code duplication.
  • Added enhanced telemetry logging for better debugging and monitoring.
  • Maintained current functionality without introducing breaking changes.

Impact of Change

  • This is a breaking change.

No breaking changes were introduced. Existing commands and workflows retain their behavior but benefit from shared logic and improved error handling.


Screenshots or Videos (if applicable)

N/A

@samikay101 samikay101 changed the base branch from main to developer/UnitTestCodeful January 21, 2025 20:06
@ccastrotrejo ccastrotrejo requested a review from Copilot January 21, 2025 20:24

Choose a reason for hiding this comment

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

Copilot reviewed 6 out of 7 changed files in this pull request and generated no comments.

Files not reviewed (1)
  • apps/vs-code-designer/src/app/utils/codeful/saveBlankUnitTest.test.ts: Evaluated as low risk
Comments suppressed due to low confidence (5)

apps/vs-code-designer/src/app/commands/workflows/unitTest/createUnitTest.ts:82

  • [nitpick] The function name 'generateUnitTestFromRun' is ambiguous. It should be renamed to 'generateUnitTestFromRunId' for clarity.
async function generateUnitTestFromRun(

apps/vs-code-designer/src/app/commands/workflows/unitTest/createUnitTest.ts:127

  • The error handling for the API call response is missing. Detailed error handling should be added to handle different error scenarios.
const response = await axios.post(apiUrl, unitTestGenerationInput, {

apps/vs-code-designer/src/app/commands/workflows/unitTest/createUnitTest.ts:151

  • [nitpick] The localized message for adding the tests directory to the workspace could be more descriptive.
await ensureDirectoryInWorkspace(paths.testsDirectory);

apps/vs-code-designer/src/app/commands/workflows/unitTest/createUnitTest.ts:82

  • The error handling logic in the 'generateUnitTestFromRun' function is not covered by tests. Add test cases to verify the error handling scenarios.
async function generateUnitTestFromRun(

apps/vs-code-designer/src/app/utils/unitTests.ts:408

  • [nitpick] The regular expression for runId validation might be too restrictive. Consider allowing lowercase letters if applicable.
const runIdFormat = /^[A-Z0-9]+$/;
apps/vs-code-designer/test-setup.ts Outdated Show resolved Hide resolved
apps/vs-code-designer/test-setup.ts Show resolved Hide resolved
@samikay101 samikay101 requested a review from hartra344 as a code owner January 24, 2025 16:16
@ccastrotrejo ccastrotrejo merged commit 6cda6c0 into Azure:developer/UnitTestCodeful Jan 27, 2025
2 checks passed
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.

3 participants