Skip to content

Fix CI build and test errors #952

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.

Sign up for GitHub

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

Merged
hotlong merged 3 commits into from
Mar 22, 2026
Merged

Fix CI build and test errors #952

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3d2eaa6
Initial plan
Copilot Mar 22, 2026
f9d9570
fix: resolve CI build and test errors
Copilot Mar 22, 2026
a9ffb71
docs: update CHANGELOG.md with CI fix details
Copilot Mar 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
remains lazy (cold-start only) via `ensureApp()` / `ensureKernel()` in `_kernel.ts`.

### Fixed
- **Service-analytics build error (TS6133)** — Removed unused `measure` variable in
`native-sql-strategy.ts` that caused the DTS build to fail with `noUnusedLocals` enabled,
blocking the entire CI build pipeline.
- **Next.js adapter test failures** — Updated 9 metadata API test assertions to match the
current `dispatch(method, path, body, queryParams, context)` call signature used by the
implementation. Tests were still expecting the old `dispatch(subPath, context, method, body)`
signature.
- **Auth plugin test failures** — Fixed 2 tests in `auth-plugin.test.ts` that referenced the
wrong `AuthManager` instance via `registerService.mock.calls`. Added `mockClear()` before
local plugin init to ensure `mock.calls[0]` points to the correct AuthManager for the test's
plugin instance.
- **SvelteKit adapter test failures** — Updated test mock to include `dispatch()` method and
aligned Metadata, Data, Error handling, and toResponse test assertions with the unified
catch-all dispatch pattern used by the implementation and all other adapters (e.g. Hono).
Expand Down
45 changes: 27 additions & 18 deletions packages/adapters/nextjs/src/metadata-api.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,10 +127,11 @@ describe('Next.js Metadata API Integration Tests', () => {
expect(res.status).toBe(200);
expect(res.body.data).toHaveLength(2);
expect(mockDispatcher.dispatch).toHaveBeenCalledWith(
'objects',
expect.objectContaining({ request: expect.anything() }),
'GET',
'/meta/objects',
undefined,
{},
expect.objectContaining({ request: expect.anything() }),
Comment on lines 130 to +134
Copy link

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

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

The 4th arg to dispatch() is the parsed query params object. Asserting it equals {} is a bit brittle and is inconsistent with packages/adapters/nextjs/src/nextjs.test.ts, which uses expect.any(Object) for that argument. Consider using expect.any(Object) (or expect.objectContaining({})) here to keep the test resilient if query parsing changes (e.g. default params added).

Copilot uses AI. Check for mistakes.
);
});
});
Expand All @@ -153,10 +154,11 @@ describe('Next.js Metadata API Integration Tests', () => {
expect(res.status).toBe(200);
expect(res.body.data.name).toBe('account');
expect(mockDispatcher.dispatch).toHaveBeenCalledWith(
'objects/account',
expect.objectContaining({ request: expect.anything() }),
'GET',
'/meta/objects/account',
undefined,
{},
expect.objectContaining({ request: expect.anything() }),
);
Comment on lines +158 to 162
Copy link

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

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

The 4th arg to dispatch() is the query params object; asserting a literal {} is brittle and differs from the pattern in packages/adapters/nextjs/src/nextjs.test.ts (uses expect.any(Object)). Consider loosening this assertion to avoid future failures if query parsing adds defaults.

Copilot uses AI. Check for mistakes.
});
});
Expand All @@ -178,10 +180,11 @@ describe('Next.js Metadata API Integration Tests', () => {
const res = await handler(req, { params: { objectstack: ['meta', 'objects'] } });
expect(res.status).toBe(201);
expect(mockDispatcher.dispatch).toHaveBeenCalledWith(
'objects',
expect.objectContaining({ request: expect.anything() }),
'POST',
'/meta/objects',
body,
{},
expect.objectContaining({ request: expect.anything() }),
);
Comment on lines +184 to 188
Copy link

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

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

Same as above: using a literal {} for the query params arg makes the assertion more fragile than necessary. Prefer expect.any(Object) / expect.objectContaining({}) like other Next.js adapter tests.

Copilot uses AI. Check for mistakes.
});
});
Expand All @@ -199,10 +202,11 @@ describe('Next.js Metadata API Integration Tests', () => {
const res = await handler(req, { params: { objectstack: ['meta', 'objects', 'account'] } });
expect(res.status).toBe(200);
expect(mockDispatcher.dispatch).toHaveBeenCalledWith(
'objects/account',
expect.objectContaining({ request: expect.anything() }),
'PUT',
'/meta/objects/account',
body,
{},
expect.objectContaining({ request: expect.anything() }),
);
Comment on lines +206 to 210
Copy link

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

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

Same concern: the query params argument is asserted as {}. Using expect.any(Object) would keep this test consistent with other adapter tests and less sensitive to internal query parsing changes.

Copilot uses AI. Check for mistakes.
});
});
Expand All @@ -229,32 +233,35 @@ describe('Next.js Metadata API Integration Tests', () => {
const req = makeReq('http://localhost/api/meta/views');
await handler(req, { params: { objectstack: ['meta', 'views'] } });
expect(mockDispatcher.dispatch).toHaveBeenCalledWith(
'views',
expect.objectContaining({ request: expect.anything() }),
'GET',
'/meta/views',
undefined,
{},
expect.objectContaining({ request: expect.anything() }),
);
Comment on lines +237 to 241
Copy link

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

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

This assertion hard-codes the query params arg to {}. Consider loosening it to expect.any(Object) / expect.objectContaining({}) to avoid brittleness if the adapter later adds default query keys.

Copilot uses AI. Check for mistakes.
});

it('dispatches for flows', async () => {
const req = makeReq('http://localhost/api/meta/flows');
await handler(req, { params: { objectstack: ['meta', 'flows'] } });
expect(mockDispatcher.dispatch).toHaveBeenCalledWith(
'flows',
expect.objectContaining({ request: expect.anything() }),
'GET',
'/meta/flows',
undefined,
{},
expect.objectContaining({ request: expect.anything() }),
);
Comment on lines +249 to 253
Copy link

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

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

This test asserts the query params argument is exactly {}. For consistency with nextjs.test.ts and to reduce brittleness, prefer expect.any(Object) / expect.objectContaining({}).

Copilot uses AI. Check for mistakes.
});

it('dispatches for agents', async () => {
const req = makeReq('http://localhost/api/meta/agents');
await handler(req, { params: { objectstack: ['meta', 'agents'] } });
expect(mockDispatcher.dispatch).toHaveBeenCalledWith(
'agents',
expect.objectContaining({ request: expect.anything() }),
'GET',
'/meta/agents',
undefined,
{},
expect.objectContaining({ request: expect.anything() }),
);
Comment on lines +261 to 265
Copy link

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

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

Query params are asserted as a literal {} here; consider expect.any(Object) / expect.objectContaining({}) to keep this test less coupled to internal query parsing details and aligned with other Next.js adapter tests.

Copilot uses AI. Check for mistakes.
});
});
Expand Down Expand Up @@ -684,10 +691,11 @@ describe('Next.js Metadata API Integration Tests', () => {
const req = makeReq('http://localhost/api/meta/objects/account/fields/name');
await handler(req, { params: { objectstack: ['meta', 'objects', 'account', 'fields', 'name'] } });
expect(mockDispatcher.dispatch).toHaveBeenCalledWith(
'objects/account/fields/name',
expect.any(Object),
'GET',
'/meta/objects/account/fields/name',
undefined,
{},
expect.objectContaining({ request: expect.anything() }),
);
Comment on lines +695 to 699
Copy link

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

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

This assertion fixes the dispatch signature, but it also hard-codes query params to {}. Using expect.any(Object) (as in packages/adapters/nextjs/src/nextjs.test.ts) makes the test more resilient if query parsing behavior changes.

Copilot uses AI. Check for mistakes.
});

Expand All @@ -696,10 +704,11 @@ describe('Next.js Metadata API Integration Tests', () => {
// With just ['meta'], subPath becomes empty after slice(1)
await handler(req, { params: { objectstack: ['meta'] } });
expect(mockDispatcher.dispatch).toHaveBeenCalledWith(
'',
expect.any(Object),
'GET',
'/meta',
undefined,
{},
expect.objectContaining({ request: expect.anything() }),
);
Comment on lines +708 to 712
Copy link

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

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

Same as the other dispatch assertions: the query params argument is asserted as {}. Consider loosening this to expect.any(Object) / expect.objectContaining({}) to avoid test brittleness and keep consistency with other adapter tests.

Copilot uses AI. Check for mistakes.
});
});
Expand Down
6 changes: 4 additions & 2 deletions packages/plugins/plugin-auth/src/auth-plugin.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,7 @@ describe('AuthPlugin', () => {
baseUrl: 'http://localhost:3000',
});
mockContext.hook = localHookCapture.hookFn;
(mockContext.registerService as any).mockClear();
await localPlugin.init(mockContext);

const mockRawApp = { all: vi.fn() };
Expand All @@ -314,7 +315,7 @@ describe('AuthPlugin', () => {
throw new Error(`Service not found: ${name}`);
});

const registeredAuthManager = (mockContext.registerService as any).mock.calls.at(-1)[1];
const registeredAuthManager = (mockContext.registerService as any).mock.calls[0][1];
const setRuntimeSpy = vi.spyOn(registeredAuthManager, 'setRuntimeBaseUrl');

Comment on lines +318 to 320
Copy link

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

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

Indexing into registerService.mock.calls[0][1] still relies on the ordering of init() registrations (currently auth first, then app.com.objectstack.system). To make the test more robust against future ordering changes, consider locating the call by service name (first arg === 'auth') rather than relying on [0].

Copilot uses AI. Check for mistakes.
await localPlugin.start(mockContext);
Expand All @@ -330,6 +331,7 @@ describe('AuthPlugin', () => {
secret: 'test-secret-at-least-32-chars-long',
});
mockContext.hook = localHookCapture.hookFn;
(mockContext.registerService as any).mockClear();
await localPlugin.init(mockContext);

const mockRawApp = { all: vi.fn() };
Expand All @@ -345,7 +347,7 @@ describe('AuthPlugin', () => {
throw new Error(`Service not found: ${name}`);
});

const registeredAuthManager = (mockContext.registerService as any).mock.calls.at(-1)[1];
const registeredAuthManager = (mockContext.registerService as any).mock.calls[0][1];
const setRuntimeSpy = vi.spyOn(registeredAuthManager, 'setRuntimeBaseUrl');

Comment on lines +350 to 352
Copy link

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

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

Same as above: relying on registerService.mock.calls[0] assumes init() always registers the AuthManager first. Consider selecting the call where the first argument is 'auth' to keep this test resilient if service registration order changes.

Copilot uses AI. Check for mistakes.
await localPlugin.start(mockContext);
Expand Down
2 changes: 0 additions & 2 deletions packages/services/service-analytics/src/strategies/native-sql-strategy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,8 +176,6 @@ export class NativeSQLStrategy implements AnalyticsStrategy {
}
if (query.measures) {
for (const m of query.measures) {
const fieldName = m.includes('.') ? m.split('.')[1] : m;
const measure = cube.measures[fieldName];
fields.push({ name: m, type: 'number' });
}
}
Expand Down
Loading