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

Improve Validation and Logging Behavior to Prevent Post-Test Logs #70

Open
jtroussard opened this issue Dec 6, 2024 · 0 comments
Open
Labels
debt enhancement New feature or request testing

Comments

@jtroussard
Copy link
Owner

Description

Currently, the validateResource method and related functions (generateTree, generateTreeEverything) produce unexpected log outputs after tests complete.

    ✔ should return normalized path if resource is Windows style path
  24 passing (24ms)
Resource is null
Invalid path data type: object
Path does not exist: /invalid/path
[main 2024-12-06T17:48:51.776Z] Extension host with pid 24743 exited with code: 0, signal: unknown.
[main 2024-12-06T17:48:51.828Z] [UtilityProcessWorker]: terminated unexpectedly with code 33052544, signal: unknown
Exit code:   0

These logs, while not causing test failures, make debugging more difficult and could indicate potential continuity issues during runtime. This behavior stems from:

  1. Synchronous Logging:

    • console.error and vscode.window.showErrorMessage are invoked directly in the validation logic, producing output even when tests pass.
  2. Centralized Error Handling:

    • Error logging and validation logic are tightly coupled in the validateResource method. This redundancy leads to logs emitted in multiple places, complicating output interpretation.
  3. Asynchronous VS Code API Calls:

    • Calls like vscode.window.showErrorMessage may queue asynchronous operations that complete after test results are reported, causing logs to appear after the test framework considers execution complete.

Proposed Solution

Refactor the validateResource method and calling functions to:

  1. Centralize Error Handling:

    • Move error handling (e.g., logging and showing error messages) to the calling functions (generateTree and generateTreeEverything) instead of directly within validateResource.
  2. Throw Exceptions Instead of Logging:

    • Modify validateResource to throw exceptions for invalid input or state, enabling the caller to handle errors consistently:
      function validateResource(resource) {
        if (!resource) throw new Error('Resource is null');
        if (typeof resource.fsPath !== 'string') throw new Error(`Invalid path data type: ${typeof resource.fsPath}`);
        const normalizedPath = normalizeToUnixStyle(resource.fsPath);
        if (!fs.existsSync(normalizedPath)) throw new Error(`Path does not exist: ${normalizedPath}`);
        return normalizedPath;
      }
  3. Mock VS Code API Calls During Tests:

    • Replace vscode.window.showErrorMessage and similar UI calls with mocks in the test suite to avoid asynchronous behavior affecting test output.
  4. Ensure Cleanup After Tests:

    • Use afterEach hooks to ensure no pending promises or tasks remain after tests:
      afterEach(() => {
        return new Promise((resolve) => setImmediate(resolve));
      });

Benefits

  • Prevents logs from appearing after test results, ensuring cleaner CI/CD pipelines and more predictable debugging.
  • Improves maintainability by separating concerns: validation vs. error handling.
  • Reduces noise and asynchronous side effects in the test environment.

Challenges

  • Refactoring these methods will require rewriting associated test cases to:
    • Handle exceptions instead of relying on returned null values.
    • Validate that errors are correctly logged by the calling functions.
  • Delays the release as these changes require significant test adjustments.

Priority

  • Low: Since tests are currently passing and the behavior does not affect functionality, this issue can be addressed in a future release cycle.

Tasks

  1. Refactor validateResource to throw exceptions for validation failures.
  2. Update generateTree and generateTreeEverything to handle exceptions.
  3. Replace vscode.window.showErrorMessage with mocks in the test suite.
  4. Add afterEach hooks to ensure no pending asynchronous operations remain after tests.
  5. Rewrite affected test cases to reflect the new behavior.

Acceptance Criteria

  • All validation logs appear in the correct order during runtime and tests.
  • Tests verify logging and error handling behavior in the calling functions.
  • No additional logs are emitted after test results during CI/CD runs.
  • Existing functionality remains intact, with all tests passing.

Milestone

  • Targeted for Post-Release Patch or Minor Version Update.
@jtroussard jtroussard added enhancement New feature or request testing debt labels Dec 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
debt enhancement New feature or request testing
Projects
None yet
Development

No branches or pull requests

1 participant