Skip to content

Conversation

@xavidop
Copy link
Owner

@xavidop xavidop commented Dec 16, 2025

...

...

...

Summary by CodeRabbit

  • New Features

    • Added newSessionPerTest configuration option to manage session behavior across test suites. When enabled, each test receives an isolated session with a fresh context; when disabled (default), tests share a single session throughout the suite.
  • Documentation

    • Updated documentation with session management guidance explaining the new configuration option and best practices for different testing scenarios.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Dec 16, 2025

Walkthrough

This PR introduces a newSessionPerTest feature enabling optional per-test session isolation. The field is added to JSON schemas, API specifications, Go type definitions, execution logic, and documentation. When enabled, each test receives a fresh user session instead of sharing one suite-wide session.

Changes

Cohort / File(s) Summary
API & Schema Definitions
docs/docs/static/conversationsuite.json, docs/docs/static/swagger.json, docs/docs/static/swagger.yaml
Added newSessionPerTest / new_session_per_test boolean field to Suite and TestSuiteRequest definitions in JSON and YAML schemas
Type Definitions
internal/types/tests/suiteTypes.go, server/handlers/handlers.go
Added NewSessionPerTest bool field with yaml/json tags to Suite and TestSuiteRequest structs
Documentation
docs/docs/test-platform/test-suites.md, docs/docs/tests/suites.md, examples/suite.yaml
Added Session Management sections explaining field behavior (default false = shared session, true = isolated sessions per test) and configuration examples
Execution Logic
pkg/test/execute.go
Implemented session generation logic: prefixes user IDs with "test-", generates fresh user ID per test when NewSessionPerTest is enabled, updated logging for both HTTP and file-based execution paths
Generated Documentation
server/docs/docs.go
Updated generated Swagger/OpenAPI documentation to include new_session_per_test field in TestSuiteRequest

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • pkg/test/execute.go – Review session generation logic, user ID prefixing, and parallel handling across HTTP vs. file-based execution paths to ensure session isolation works correctly
  • Field propagation – Verify NewSessionPerTest flows correctly from handlers through to execution layer
  • Logging consistency – Confirm per-test vs. per-suite session logging is accurate and helpful

Poem

🐰 A session per test, oh what a quest!
Fresh IDs bloom, each test gets rest,
No context shared, no state to jest,
Isolation rules—your tests are blessed!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat: new session per test execution' directly and clearly describes the main change: adding a new session per test feature for test execution.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch xavier/new-session-per-request

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 684c1dc and e69c4e3.

📒 Files selected for processing (10)
  • docs/docs/static/conversationsuite.json (1 hunks)
  • docs/docs/static/swagger.json (1 hunks)
  • docs/docs/static/swagger.yaml (1 hunks)
  • docs/docs/test-platform/test-suites.md (2 hunks)
  • docs/docs/tests/suites.md (2 hunks)
  • examples/suite.yaml (1 hunks)
  • internal/types/tests/suiteTypes.go (1 hunks)
  • pkg/test/execute.go (5 hunks)
  • server/docs/docs.go (1 hunks)
  • server/handlers/handlers.go (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
internal/types/tests/suiteTypes.go (1)
internal/types/tests/testTypes.go (1)
  • OpenAIConfig (97-100)
server/handlers/handlers.go (1)
internal/types/tests/suiteTypes.go (1)
  • Suite (3-10)
pkg/test/execute.go (1)
internal/global/vars.go (1)
  • Log (5-5)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Summary
🔇 Additional comments (17)
docs/docs/static/swagger.yaml (1)

101-103: LGTM!

The new_session_per_test property is correctly added with snake_case naming consistent with other API properties (e.g., environment_name), and the default example of false aligns with the documented behavior.

server/docs/docs.go (1)

307-310: LGTM!

The generated Swagger definition correctly includes the new_session_per_test property with consistent snake_case naming and boolean type.

examples/suite.yaml (1)

4-6: LGTM!

Good documentation with clear comments explaining the optional nature and default behavior. Setting newSessionPerTest: true in the example appropriately demonstrates the new feature.

docs/docs/static/conversationsuite.json (1)

29-31: LGTM!

The newSessionPerTest property is correctly added to the Suite schema with camelCase naming consistent with other properties like environmentName.

docs/docs/test-platform/test-suites.md (1)

80-110: LGTM!

Comprehensive documentation with clear explanations of session behavior for both settings. The JSON example correctly uses new_session_per_test (snake_case) matching the API schema.

docs/docs/static/swagger.json (1)

301-304: LGTM!

The new_session_per_test property is correctly added with snake_case naming and example value consistent with the YAML specification.

internal/types/tests/suiteTypes.go (1)

3-10: LGTM!

The NewSessionPerTest field is correctly added with omitempty tags for optional serialization. The camelCase naming in YAML/JSON tags (newSessionPerTest) aligns with the internal schema, while the API layer uses snake_case (new_session_per_test). Go's JSON marshaling automatically handles the conversion between these naming conventions through struct tags.

docs/docs/tests/suites.md (2)

16-18: LGTM! Clear documentation of the new field.

The documentation clearly explains the optional field, its default value, and its behavior. Showing the default value explicitly in the example is good practice.


29-42: LGTM! Excellent documentation section.

The Session Management section clearly explains both modes of operation (shared vs. isolated sessions) with concrete implications for variables, context, and test execution. The structure and content are clear and helpful for users.

server/handlers/handlers.go (2)

29-29: LGTM! Field properly defined.

The NewSessionPerTest field is correctly defined with appropriate JSON tag, example value, and positioning within the struct. The snake_case convention is consistent with other fields like environment_name.


191-191: LGTM! Field properly propagated.

The NewSessionPerTest flag is correctly propagated from the HTTP request to the test execution layer, ensuring the per-test session behavior is applied during execution.

pkg/test/execute.go (6)

17-17: LGTM! Field properly defined with clear documentation.

The NewSessionPerTest field is correctly defined with appropriate JSON tag, omitempty for optional behavior, and a clear inline comment explaining its purpose and default value.


72-72: LGTM! Test session prefix improves clarity.

Adding the "test-" prefix to generated user IDs helps distinguish test sessions from production sessions, improving traceability and debugging.


81-85: LGTM! Clear conditional logging.

The conditional logging appropriately handles both modes: logging the session status when per-test sessions are enabled, or logging the shared user ID when using a single session. This provides clear visibility into the test execution behavior.


90-94: LGTM! Per-test session generation correctly implemented.

The per-test session logic is properly placed within the test loop and generates a fresh user ID for each test when enabled. The logging clearly indicates which user ID is being used for each test, improving traceability.


110-110: LGTM! Consistent implementation across execution paths.

The file-based execution path mirrors the HTTP execution path with the same "test-" prefix and conditional logging pattern, ensuring consistent behavior regardless of how tests are executed.

Also applies to: 121-125


129-133: LGTM! File-based execution properly implements per-test sessions.

The per-test session logic for file-based execution mirrors the HTTP path, ensuring consistent behavior. The logging uses the appropriate test file ID for clarity.


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

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

@mergify mergify bot added the size/L label Dec 16, 2025
@xavidop xavidop merged commit e0c8e9b into main Dec 16, 2025
9 of 10 checks passed
@xavidop xavidop deleted the xavier/new-session-per-request branch December 16, 2025 11:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants