Skip to content

chore: rm duplicate test#123

Merged
infiniteregrets merged 2 commits intomainfrom
m/test-updates-2
Feb 5, 2026
Merged

chore: rm duplicate test#123
infiniteregrets merged 2 commits intomainfrom
m/test-updates-2

Conversation

@infiniteregrets
Copy link
Member

No description provided.

@infiniteregrets infiniteregrets requested a review from a team as a code owner February 5, 2026 15:14
@greptile-apps
Copy link

greptile-apps bot commented Feb 5, 2026

Greptile Overview

Greptile Summary

This PR removes duplicate test cases from spec.e2e.test.ts that were already covered in accessTokens.spec.e2e.test.ts, cleaning up the test suite organization.

Key changes:

  • Removed duplicate access token auto-prefix tests from spec.e2e.test.ts (lines 329-385)
  • Removed metrics validation tests from spec.e2e.test.ts (lines 387-514) - these appear to have been moved or consolidated elsewhere
  • Added the "auto-prefixes streams when enabled" test to accessTokens.spec.e2e.test.ts with a critical bug fix: the assertion now correctly expects prefixedName instead of rawName (line 353), addressing the issue noted in the previous review thread
  • Cleaned up unused variables (endpoints, metricsStreamName) from spec.e2e.test.ts
  • Updated CI workflow to remove --exclude '**/spec.e2e*' from the S2-lite integration test command, allowing spec.e2e.test.ts to run again

The bug fix in the moved test is particularly important: when autoPrefixStreams is enabled, the limited client should see streams with the enforced prefix, not the raw names. The corrected assertion validates this expected behavior.

Confidence Score: 5/5

  • This PR is safe to merge - it improves test organization and fixes a test assertion bug
  • The changes are straightforward test reorganization with no functional code changes. The PR removes duplicate tests, consolidates them in the appropriate file, fixes a bug in the test assertion (expecting prefixedName instead of rawName), and updates CI configuration accordingly. All changes improve code quality and test correctness.
  • No files require special attention

Important Files Changed

Filename Overview
.github/workflows/ci.yml Removed --exclude '**/spec.e2e*' from S2-lite integration test command
packages/streamstore/src/tests/accessTokens.spec.e2e.test.ts Added auto-prefixes streams when enabled test with corrected assertion that expects prefixedName instead of rawName
packages/streamstore/src/tests/spec.e2e.test.ts Removed duplicate access token auto-prefix tests and metrics validation tests, cleaned up unused variables (endpoints, metricsStreamName)

Sequence Diagram

sequenceDiagram
    participant Dev as Developer
    participant CI as CI Workflow
    participant Spec as spec.e2e.test.ts
    participant Access as accessTokens.spec.e2e.test.ts
    
    Note over Dev,Access: PR removes duplicate tests from spec.e2e.test.ts
    
    Dev->>Spec: Remove duplicate access token tests
    Dev->>Spec: Remove metrics validation tests
    Dev->>Spec: Clean up unused variables (endpoints, metricsStreamName)
    
    Dev->>Access: Add "auto-prefixes streams when enabled" test
    Note over Access: Test now correctly expects prefixedName<br/>instead of rawName (bug fix)
    
    Dev->>CI: Update S2-lite integration test command
    Note over CI: Remove --exclude '**/spec.e2e*'<br/>to allow spec.e2e.test.ts to run
    
    CI->>Spec: Run remaining tests (read semantics, auto-create)
    CI->>Access: Run access token tests including new test
Loading

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

3 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@infiniteregrets
Copy link
Member Author

@greptile-apps review

@infiniteregrets infiniteregrets merged commit bf8842a into main Feb 5, 2026
6 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.

1 participant

Comments