refactor(logger): extract TestContext types and rename test context module

[xinredhat]

User description

This refactoring improves code organization by separating type definitions from implementation, making the codebase more maintainable and imports more explicit.

Co-Authored-By: Claude Sonnet 4.5

---

PR Type

Enhancement

---

Description

• Extract TestContext type definitions into dedicated types module

• Rename testContext module to testContextStorage for clarity

• Update all imports across codebase to reflect new module structure

• Simplify context manager fallback logic by removing dynamic require

---

Diagram Walkthrough

flowchart LR
  A["testContext.ts<br/>Old Module"] -->|"Extract Types"| B["types.ts<br/>Type Definitions"]
  A -->|"Rename to"| C["testContextStorage.ts<br/>Storage Implementation"]
  B -->|"Import"| D["contextManager.ts"]
  B -->|"Import"| E["AsyncContextInjector.ts"]
  B -->|"Import"| F["Formatters<br/>jsonFormatter, textFormatter"]
  C -->|"Import"| G["fixtures.ts"]

Loading

File Walkthrough

Relevant files

Enhancement

types.ts Create dedicated TestContext types module                                src/logger/context/types.ts New file created to hold TestContext interface definition Contains documented interface with projectName and worker properties Includes comprehensive JSDoc explaining auto-injected context fields +10/-0    testContextStorage.ts Remove type definitions, simplify imports                                src/logger/context/testContextStorage.ts Remove TestContext interface definition (moved to types.ts) Add direct import of contextManager instead of dynamic require Replace try-catch dynamic require with direct function call in getCurrentTestContext Update imports to use new types module +3/-18    contextManager.ts Update TestContext import path                                                      src/logger/context/contextManager.ts Update import statement to reference types module instead of testContext +1/-1      AsyncContextInjector.ts Update context import path                                                              src/logger/features/context/AsyncContextInjector.ts Update import to use testContextStorage module instead of testContext +1/-1      jsonFormatter.ts Update context import path                                                              src/logger/formatters/jsonFormatter.ts Update import to use testContextStorage module instead of testContext +1/-1      textFormatter.ts Update context import path                                                              src/logger/formatters/textFormatter.ts Update import to use testContextStorage module instead of testContext +1/-1      metadata.types.ts Update TestContext import source                                                  src/logger/types/metadata.types.ts Update TestContext import to reference new types module Maintain re-export of TestContext for convenience +1/-1      fixtures.ts Update context import path                                                              src/utils/test/fixtures.ts Update imports to use testContextStorage module instead of testContext +1/-1

---

Summary by CodeRabbit

• Refactor
  • Reorganized internal type definitions for test context metadata to improve code structure and maintainability.
  • Consolidated test context interface definitions into a centralized types module.
  • Simplified test context retrieval logic by removing redundant fallback mechanisms.

[coderabbitai]

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

📝 Walkthrough

Walkthrough

This pull request refactors the test context architecture in the logger system by centralizing the TestContext interface definition into a dedicated types.ts file and updating all import paths across the codebase to reference the new location. No changes to public API or runtime behavior occur.

Changes

Cohort / File(s)	Summary
Type Definition Consolidation src/logger/context/types.ts, src/logger/context/testContextStorage.ts	Centralized TestContext interface definition in types.ts with projectName and worker properties. testContextStorage.ts now imports from types instead of declaring locally; simplified getCurrentTestContext() implementation by removing dynamic fallback logic.
Module Import Updates src/logger/context/contextManager.ts, src/logger/types/metadata.types.ts, src/logger/formatters/jsonFormatter.ts, src/logger/formatters/textFormatter.ts, src/logger/features/context/AsyncContextInjector.ts, src/utils/test/fixtures.ts	Updated import paths for TestContext and getCurrentTestContext to reference new centralized locations (types.ts and testContextStorage.ts).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

enhancement, testing

Suggested reviewers

• xinredhat
• jsmid1
• Katka92

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically describes the main refactoring work: extracting TestContext types into a dedicated module and renaming the test context module.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Removed fallback guard: The previous guarded dynamic import with a safe undefined fallback was replaced by a direct contextManager.getContext() call, which could turn module-load or dependency issues into hard failures instead of graceful degradation. Referred Code export function getCurrentTestContext(): TestContext | undefined { // Try AsyncLocalStorage first const alsContext = testContextStorage.getStore(); if (alsContext) { return alsContext; } // Fallback to global context manager (for when AsyncLocalStorage doesn't propagate) return contextManager.getContext(); } Learn more about managing compliance generic rules or creating your own custom rules
Update

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Lazy-load and guard contextManager call To prevent circular dependency issues, move the contextManager import into the getCurrentTestContext function, using a dynamic require within a try-catch block for safe, lazy loading. src/logger/context/testContextStorage.ts [8-42] -import { contextManager } from './contextManager'; +// remove top‐level import of contextManager ... -return contextManager.getContext(); +export function getCurrentTestContext(): TestContext | undefined { + const alsContext = testContextStorage.getStore(); + if (alsContext) { + return alsContext; + } + // Lazy‐load and safely call contextManager + try { + const { contextManager } = require('./contextManager'); + return contextManager.getContext(); + } catch { + return undefined; + } +} + [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 9 __ Why: This suggestion correctly identifies a critical circular dependency issue introduced by the PR and proposes reverting to the original, safer pattern of using a dynamic require within a try-catch block, which prevents potential runtime errors.	High
Update

[qodo-code-review]

Review Summary by Qodo

Extract TestContext types to dedicated module

✨ Enhancement

Walkthroughs

Description

• Extract TestContext interface to dedicated types.ts module
• Update imports across codebase to reference new types location
• Simplify context manager fallback logic by removing dynamic require
• Improve code organization and maintainability through separation of concerns

Diagram

flowchart LR
  old["testContext.ts<br/>Types + Implementation"]
  types["types.ts<br/>TestContext Interface"]
  storage["testContextStorage.ts<br/>Implementation"]
  imports["Updated Imports<br/>Across Codebase"]
  old -- "Extract Types" --> types
  old -- "Keep Implementation" --> storage
  types -- "Referenced By" --> imports
  storage -- "Updated To" --> imports

Loading

File Changes

1. src/logger/context/types.ts ✨ Enhancement +10/-0

New TestContext type definitions module

• New file created to house TestContext interface definition
• Contains comprehensive JSDoc documentation with examples
• Defines optional projectName and worker properties

src/logger/context/types.ts

---

2. src/logger/context/testContextStorage.ts ✨ Enhancement +3/-18

Remove types, simplify context retrieval logic

• Removed TestContext interface definition (moved to types.ts)
• Updated imports to use new types.ts module
• Simplified getCurrentTestContext() by removing try-catch dynamic require
• Now directly imports contextManager at module level

src/logger/context/testContextStorage.ts

---

3. src/logger/context/contextManager.ts ✨ Enhancement +1/-1

Update TestContext import path

• Updated import statement to reference TestContext from ./types
• No functional changes to implementation

src/logger/context/contextManager.ts

---

View more (5)

4. src/logger/features/context/AsyncContextInjector.ts ✨ Enhancement +1/-1

Fix import path for context function

• Updated import path for getCurrentTestContext from testContext to testContextStorage
• Corrects import to reference actual implementation module

src/logger/features/context/AsyncContextInjector.ts

---

5. src/logger/formatters/jsonFormatter.ts ✨ Enhancement +1/-1

Update context import path

• Updated import path for getCurrentTestContext from testContext to testContextStorage
• Aligns with new module organization

src/logger/formatters/jsonFormatter.ts

---

6. src/logger/formatters/textFormatter.ts ✨ Enhancement +1/-1

Update context import path

• Updated import path for getCurrentTestContext from testContext to testContextStorage
• Maintains consistency with new module structure

src/logger/formatters/textFormatter.ts

---

7. src/logger/types/metadata.types.ts ✨ Enhancement +1/-1

Update TestContext import source

• Updated import to reference TestContext from ../context/types
• Re-exports TestContext for convenience

src/logger/types/metadata.types.ts

---

8. src/utils/test/fixtures.ts ✨ Enhancement +1/-1

Update context storage import path

• Updated import path for testContextStorage and extractTestContext from testContext to
 testContextStorage
• Reflects new module organization

src/utils/test/fixtures.ts

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0)

1. Playwright dep leaks 🐞 Bug ⛯ Reliability

Description

• jsonFormatter (and other core logger components) now import getCurrentTestContext from
  testContextStorage, making the core logger dependency graph include testContextStorage.
• testContextStorage.ts has a value import of TestInfo from @playwright/test even though it’s
  only used for typing; in some runtime/transpile/bundle setups this can become a real runtime
  require('@playwright/test'), breaking non-test utilities/scripts that use the logger when
  devDependencies aren’t installed.
• This is higher-risk because LoggerFactory.getLogger() always instantiates
  AsyncContextInjector, so the context chain is on the hot path for all logging, not just Playwright
  fixtures.

Code

src/logger/formatters/jsonFormatter.ts[14]

+import { getCurrentTestContext } from '../context/testContextStorage';

Evidence

The PR changes core logger formatters/injectors to import from testContextStorage. That module
imports @playwright/test (devDependency) using a non-type-only import, so logger usage can
unexpectedly require Playwright at runtime depending on transpilation/bundling. LoggerFactory always
creates AsyncContextInjector, so this path is always active when creating loggers.

src/logger/formatters/jsonFormatter.ts[13-15]
src/logger/features/context/AsyncContextInjector.ts[8-10]
src/logger/context/testContextStorage.ts[6-9]
src/logger/context/testContextStorage.ts[21-25]
src/logger/factory/loggerFactory.ts[102-107]
package.json[44-51]
src/utils/cliChecker.ts[1-9]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
Core logger modules now depend (transitively) on `src/logger/context/testContextStorage.ts`, which imports `TestInfo` from `@playwright/test` using a value import. Since `TestInfo` is only used as a type annotation, this should be a type-only import; otherwise some transpilers/bundlers can preserve a runtime dependency on `@playwright/test` (a devDependency), causing runtime failures in non-test contexts.

### Issue Context
- LoggerFactory always instantiates `AsyncContextInjector`, and formatters import `getCurrentTestContext`, so this dependency can affect all logger usage.
- `@playwright/test` is listed under `devDependencies`, increasing risk when devDeps are omitted.

### Fix Focus Areas
- src/logger/context/testContextStorage.ts[6-9]
- (optional hardening) src/logger/context/testContextStorage.ts[17-26]

### Suggested changes
1. Change `import { TestInfo } from '@playwright/test';` to `import type { TestInfo } from '@playwright/test';`.
2. (Optional, stronger decoupling) Move `extractTestContext()` (or its `TestInfo` typing) into a Playwright-only module (e.g., `src/utils/test/...`) and keep `testContextStorage` free of Playwright imports by accepting a minimal structural type instead.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

ⓘ The new review experience is currently in Beta. Learn more