Skip to content

Conversation

@MarcScheib
Copy link
Contributor

This pull request introduces a new MemoryLogger class for in-memory logging, integrates it into the workspace utilities, and adds comprehensive test coverage for workspace-related functionality. The most significant changes include the implementation of the MemoryLogger, updates to the workspace utilities to remove direct logging in certain cases, and the addition of unit tests for workspace path handling.

Logging Enhancements:

  • Added a new MemoryLogger class that extends Logger, providing in-memory storage for log messages with methods for logging, retrieving, and clearing messages (src/utils/logger.ts).

Workspace Utilities Updates:

  • Removed direct error logging from the isExistingWorkspace function to avoid side effects and improve testability (src/utils/workspace.ts).

Test Coverage:

  • Added unit tests for workspace utilities (resolveWorkspacePath, getWorkspacePathIdentifier, and isExistingWorkspace) with mock dependencies and integration of MemoryLogger for verifying logging behavior (src/utils/workspace.test.ts).

@MarcScheib MarcScheib linked an issue May 1, 2025 that may be closed by this pull request
@MarcScheib MarcScheib requested a review from Copilot May 1, 2025 16:29
Copy link

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 PR removes direct logging from the workspace directory check to avoid unwanted side effects, integrates an in-memory logger (MemoryLogger) for improved testability, and adds robust unit tests for workspace path handling.

  • Removed direct Logger error logging when the workspace directory does not exist.
  • Implemented a new MemoryLogger for in-memory logging and integrated it into workspace utilities.
  • Added unit tests to verify workspace path resolution and logging behavior for missing configuration files.

Reviewed Changes

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

File Description
src/utils/workspace.ts Removed logging from the missing directory check; retains logging for missing config file.
src/utils/workspace.test.ts Added and updated tests for the workspace utilities using MemoryLogger.
src/utils/logger.ts Introduced the MemoryLogger class extending Logger for in-memory log storage.
Comments suppressed due to low confidence (2)

src/utils/workspace.ts:40

  • [nitpick] Add a comment clarifying that the removal of logging here is intentional to avoid side effects in tests.
inject(Logger).error('The provided workspacePath "' + workspacePath + '" does not exist.');

src/utils/workspace.ts:92

  • [nitpick] Consider using a template literal for improved readability in the error message, e.g., Expected to find the file "${identifier}.json" but was not found.
inject(Logger).error('Expected to find the file "' + identifier + '.json" but was not found.');

@MarcScheib MarcScheib merged commit fefed0e into develop May 1, 2025
1 check passed
@MarcScheib MarcScheib deleted the 5-init-command-currently-logs-an-error-if-no-workspace-exists branch May 1, 2025 16:30
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.

Init command currently logs an error if no workspace exists

2 participants