Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new Vitest-based test_v2 suite (with schema-only DB bootstrap and per-file test harness) to decouple tests from legacy Jest + dummy seed data, while porting/adding a broad set of unit + API-level tests.
Changes:
- Introduce Vitest configuration and npm scripts to run the new
test_v2suite (including DB schema import). - Add
@internal/testing/helperstest harness (context, factories, snapshots, upload helpers, S3 cleanup) to make tests self-contained. - Port/author extensive
test_v2unit and API tests (buckets, objects, auth/crypto, caching, DB utilities, etc.).
Reviewed changes
Copilot reviewed 53 out of 55 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| vitest.config.ts | Adds Vitest config targeting test_v2/**/*.test.ts with forked workers and shared setup. |
| tsconfig.test_v2.json | Adds a test-focused TS config including src + test_v2. |
| tsconfig.json | Excludes src/internal/testing/helpers from the main TS build. |
| package.json | Adds test:schema and test:v2* scripts; adds Vitest + coverage dev deps. |
| test_v2/setup.ts | Vitest setup hook to load env/config per test and shutdown OTel globals. |
| test_v2/db/import-schema.ts | Script to import minimal auth schema required for RLS tests (schema-only). |
| test_v2/db/01-auth-schema.sql | Minimal auth schema + helpers (auth.uid(), auth.role()) for storage RLS. |
| test_v2/unit/xml-parser-plugin.test.ts | Unit coverage for XML parser plugin request parsing + XML response serialization. |
| test_v2/unit/validators.test.ts | Unit coverage for header validator plugin and validateXRobotsTag. |
| test_v2/unit/signature-v4-stream.test.ts | Unit coverage for chunked SigV4 stream parser edge cases and events. |
| test_v2/unit/scanner-pagination.test.ts | Regression test for scanner pagination continuation-token behavior. |
| test_v2/unit/s3-router.test.ts | Unit coverage for S3 router query matching + metadata header parsing. |
| test_v2/unit/pool-database.test.ts | DB-level test for pool acquire behavior across destroy/recreate. |
| test_v2/unit/pool-cache.test.ts | Unit coverage for PoolManager TTL cache lifecycle/monitoring behaviors under Vitest. |
| test_v2/unit/orphan-objects.test.ts | Unit coverage for orphan-client NDJSON stream writer error/edge handling. |
| test_v2/unit/operation-helpers.test.ts | DB-level tests for SQL helpers controlling allowed operations. |
| test_v2/unit/ndjson.test.ts | Unit coverage for NDJSON transform parsing and error propagation. |
| test_v2/unit/migrations-transformers.test.ts | Unit coverage for migration SQL transformer behavior. |
| test_v2/unit/migrations-memoize.test.ts | Adds memoization tests (but currently tests a local implementation). |
| test_v2/unit/log-request.test.ts | Unit coverage for request logging plugin resource derivation. |
| test_v2/unit/jwt.test.ts | Unit coverage for JWT verify/sign paths + cache behavior. |
| test_v2/unit/hash-stream.test.ts | Extensive unit coverage for HashSpillWritable spill/cleanup/concurrency behaviors. |
| test_v2/unit/error.test.ts | Unit coverage for render() behavior on plain/renderable errors. |
| test_v2/unit/database-util.test.ts | Unit coverage for DB SSL settings and IP detection utilities. |
| test_v2/unit/database-protection.test.ts | DB-level tests for delete-protection trigger behavior. |
| test_v2/unit/cache-ttl.test.ts | Unit coverage for TTL cache wrapper stats/iteration behaviors. |
| test_v2/unit/cache-metrics.test.ts | Unit coverage for cache telemetry helpers and occupancy metrics behavior. |
| test_v2/unit/cache-lru.test.ts | Unit coverage for LRU cache wrapper outcomes/stats/timers. |
| test_v2/unit/auth-crypto.test.ts | Unit coverage for auth crypto decrypt compatibility and roundtrip. |
| test_v2/unit/async-abort-controller.test.ts | Unit coverage for AsyncAbortController group reuse and abort ordering. |
| test_v2/objects/object-x-robots.test.ts | API-level tests verifying x-robots-tag roundtrip through real upload/read flow. |
| test_v2/objects/object-upload.test.ts | API-level upload tests (multipart/binary/PUT/POST) without relying on dummy seed data. |
| test_v2/objects/object-signed-urls.test.ts | API-level signed URL minting/consumption/upload-sign flows. |
| test_v2/objects/object-move.test.ts | API-level move object tests including rollback behavior on backend failure. |
| test_v2/objects/object-list.test.ts | API-level list behavior tests using DB-seeded objects for deterministic results. |
| test_v2/objects/object-get.test.ts | API-level GET/HEAD tests for private/public buckets and caching headers. |
| test_v2/objects/object-delete.test.ts | API-level delete + bulk delete tests (bulk via DB-seeded rows). |
| test_v2/objects/object-copy.test.ts | API-level copy tests (within/across buckets, metadata behaviors). |
| test_v2/buckets/search-filters.test.ts | API-level tests for wildcard escaping in bucket search filters. |
| test_v2/buckets/bucket.test.ts | API-level CRUD tests for bucket routes using the new harness. |
| src/internal/testing/helpers/wait.ts | Adds polling helper for eventually-consistent/async side effects. |
| src/internal/testing/helpers/upload.ts | Adds multipart/binary upload helpers and shared fixture constants. |
| src/internal/testing/helpers/snapshot.ts | Adds snapshot-style DB row assertions that ignore volatile fields. |
| src/internal/testing/helpers/s3.ts | Adds S3 client + root bucket ensure + teardown prefix deletion. |
| src/internal/testing/helpers/random.ts | Adds per-file prefix and unique name helpers for parallel-safe test IDs. |
| src/internal/testing/helpers/index.ts | Public barrel exports for the new test harness. |
| src/internal/testing/helpers/factories/user.ts | Adds user factory that inserts auth.users rows and mints JWTs. |
| src/internal/testing/helpers/factories/types.ts | Adds cleanup registry tracking created test resources. |
| src/internal/testing/helpers/factories/object.ts | Adds object factory for DB-seeded objects with batched inserts. |
| src/internal/testing/helpers/factories/bucket.ts | Adds bucket factory for DB-seeded buckets with common presets. |
| src/internal/testing/helpers/db.ts | Adds per-worker superuser Knex singleton + withDeleteEnabled helper. |
| src/internal/testing/helpers/context.ts | Adds useTestContext() harness wiring hooks + bulk teardown. |
| src/internal/testing/helpers/client.ts | Adds test client wrapper around Fastify inject with auth scoping. |
| src/internal/testing/helpers/auth.ts | Adds JWT mint helper + anon/service key helpers for tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| }) | ||
|
|
||
| test('returns 404 for a non-existent bucket', async () => { | ||
| const res = await ctx.client.asAnon().get(`/object/${ctx.prefix}_does_not_exist`) |
| test('non-existent bucket is rejected and nothing is created', async () => { | ||
| const name = `${ctx.prefix}_missing` | ||
| const res = await ctx.client | ||
| .unauthenticated() |
Comment on lines
+38
to
+39
| * Build an object factory bound to a bucket. Pass either the TestBucket | ||
| * returned by `factories.bucket.create()` or just its id. |
Comment on lines
+3
to
+16
| describe('memoizePromise helper', () => { | ||
| const memoizePromise = <T extends any[], R>( | ||
| fn: (...args: T) => Promise<R> | ||
| ): ((...args: T) => Promise<R>) => { | ||
| const cache = new Map<string, Promise<R>>() | ||
|
|
||
| return async (...args: T): Promise<R> => { | ||
| const key = JSON.stringify(args) | ||
| if (cache.has(key)) return cache.get(key)! | ||
| const promise = fn(...args) | ||
| cache.set(key, promise) | ||
| return promise | ||
| } | ||
| } |
Comment on lines
+4
to
+11
| export default defineConfig({ | ||
| resolve: { | ||
| tsconfigPaths: true, | ||
| alias: { | ||
| '@storage': path.resolve(__dirname, 'src/storage'), | ||
| '@internal': path.resolve(__dirname, 'src/internal'), | ||
| }, | ||
| }, |
Comment on lines
+21
to
+27
| client = new S3Client({ | ||
| endpoint: storageS3Endpoint, | ||
| region: storageS3Region, | ||
| forcePathStyle: storageS3ForcePathStyle, | ||
| credentials: { | ||
| accessKeyId: process.env.AWS_ACCESS_KEY_ID || '', | ||
| secretAccessKey: process.env.AWS_SECRET_ACCESS_KEY || '', |
Comment on lines
+8
to
+10
| describe('TenantPool Database', () => { | ||
| it('can acquire a on a destroyed pool', async () => { | ||
| const superUser = await getServiceKeyUser(tenantId) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What kind of change does this PR introduce?
Chore
What is the current behavior?
What is the new behavior?