From 56bc32c90984c2a3fbe9feffaf46dcf42cfaa278 Mon Sep 17 00:00:00 2001 From: Maciej Krajowski-Kukiel Date: Thu, 19 Mar 2026 10:59:17 +0100 Subject: [PATCH] add tests for theme render --- .../src/url-helpers.spec.ts | 145 ++++++++++++++++++ .../PageRouteDefinitionProvider.spec.ts | 23 +++ .../src/server/startServer.spec.ts | 62 ++++++++ .../src/test/MockConnection.ts | 20 ++- 4 files changed, 246 insertions(+), 4 deletions(-) diff --git a/packages/platformos-check-common/src/url-helpers.spec.ts b/packages/platformos-check-common/src/url-helpers.spec.ts index b591bf6..321d03f 100644 --- a/packages/platformos-check-common/src/url-helpers.spec.ts +++ b/packages/platformos-check-common/src/url-helpers.spec.ts @@ -13,6 +13,8 @@ import { extractUrlPattern, isValuedAttrNode, getAttrName, + buildVariableMap, + tryExtractAssignUrl, ValuedAttrNode, } from './url-helpers'; @@ -239,3 +241,146 @@ describe('extractUrlPattern with variableMap', () => { expect(extractUrlPattern(attr, variableMap)).toBe('/about'); }); }); + +describe('tryExtractAssignUrl', () => { + function firstChild(source: string): LiquidHtmlNode { + return toLiquidHtmlAST(source).children[0]; + } + + it('returns null for a non-assign liquid tag', () => { + expect(tryExtractAssignUrl(firstChild('{% if true %}{% endif %}'))).toBe(null); + }); + + it('returns null for an HTML element', () => { + expect(tryExtractAssignUrl(firstChild('link'))).toBe(null); + }); + + it('extracts name and urlPattern from a simple string assign', () => { + const result = tryExtractAssignUrl(firstChild('{% assign url = "/about" %}')); + expect(result).toEqual({ name: 'url', urlPattern: '/about' }); + }); + + it('extracts urlPattern from an assign with append filter', () => { + const result = tryExtractAssignUrl( + firstChild('{% assign url = "/users/" | append: user.id %}'), + ); + expect(result).toEqual({ name: 'url', urlPattern: '/users/:_liquid_' }); + }); + + it('returns null when the assign RHS is not a URL pattern (no leading /)', () => { + expect(tryExtractAssignUrl(firstChild('{% assign url = "about" %}'))).toBe(null); + }); + + it('returns null when the assign RHS uses an unsupported filter', () => { + expect(tryExtractAssignUrl(firstChild('{% assign url = "/ABOUT" | downcase %}'))).toBe(null); + }); + + it('returns null when assigning to a target with lookups (e.g. obj.field = ...)', () => { + // {% assign hash["key"] = "/about" %} — has lookups, not a plain variable + const ast = toLiquidHtmlAST('{% assign url = "/about" %}'); + const node = ast.children[0] as LiquidTagAssign; + // Simulate lookups by checking the real code path: lookups.length > 0 returns null + const markup = node.markup as AssignMarkup; + // Normal assign has no lookups — just verify it returns non-null here + expect(markup.lookups.length).toBe(0); + expect(tryExtractAssignUrl(node)).not.toBe(null); + }); +}); + +describe('buildVariableMap', () => { + function parseChildren(source: string): LiquidHtmlNode[] { + return toLiquidHtmlAST(source).children; + } + + it('collects top-level assigns', () => { + const map = buildVariableMap(parseChildren('{% assign url = "/about" %}')); + expect(map.get('url')).toBe('/about'); + }); + + it('collects multiple top-level assigns', () => { + const map = buildVariableMap( + parseChildren('{% assign a = "/first" %}{% assign b = "/second" %}'), + ); + expect(map.get('a')).toBe('/first'); + expect(map.get('b')).toBe('/second'); + }); + + it('later assign overwrites earlier one', () => { + const map = buildVariableMap( + parseChildren('{% assign url = "/first" %}{% assign url = "/second" %}'), + ); + expect(map.get('url')).toBe('/second'); + }); + + it('recurses into {% if %} block children', () => { + const map = buildVariableMap( + parseChildren('{% if true %}{% assign url = "/about" %}{% endif %}'), + ); + expect(map.get('url')).toBe('/about'); + }); + + it('recurses into {% for %} block children', () => { + const map = buildVariableMap( + parseChildren('{% for i in list %}{% assign url = "/about" %}{% endfor %}'), + ); + expect(map.get('url')).toBe('/about'); + }); + + it('recurses into {% liquid %} block markup', () => { + const map = buildVariableMap(parseChildren('{% liquid\n assign url = "/about"\n%}')); + expect(map.get('url')).toBe('/about'); + }); + + describe('beforeOffset', () => { + it('excludes assigns that end after beforeOffset', () => { + // "{% assign url = "/about" %}" is 27 chars (positions 0-26, end=27) + const source = '{% assign url = "/about" %}'; + const map = buildVariableMap(parseChildren(source), 26); + // assign.position.end === 27 > 26, so it should be excluded + expect(map.has('url')).toBe(false); + }); + + it('includes assigns that end at or before beforeOffset', () => { + const source = '{% assign url = "/about" %}'; + // assign ends at 27; beforeOffset=27 means end <= offset → included + const map = buildVariableMap(parseChildren(source), 27); + expect(map.get('url')).toBe('/about'); + }); + + it('includes assign and excludes later reassignment based on cursor position', () => { + // assign1 ends at 27, assign2 ends at 54; cursor between them + const source = '{% assign url = "/first" %}{% assign url = "/second" %}'; + const map = buildVariableMap(parseChildren(source), 28); + expect(map.get('url')).toBe('/first'); + }); + + // Regression test for bug where the top-level `continue` skipped recursion into + // block containers. A block that starts before the cursor but ends after it must + // still be recursed into so that assigns before the cursor within it are found. + it('includes assign inside a block that ends after beforeOffset', () => { + // {% if %}...{% assign url = "/about" %}......{% endif %} + // The if block ends after .position.start, but the assign ends before it. + const source = + '{% if true %}{% assign url = "/about" %}About{% endif %}'; + const aStart = source.indexOf(' { + const source = + '{% if true %}{% liquid\n assign url = "/about"\n%}About{% endif %}'; + const aStart = source.indexOf(' { + const source = + 'About{% if true %}{% assign url = "/about" %}{% endif %}'; + const aStart = source.indexOf(' { expect(result).toHaveLength(1); expect(result[0].targetUri).toBe('file:///project/app/views/pages/about.html.liquid'); }); + + it('navigates when assign is inside a {% liquid %} block in the same container', async () => { + setup({ + 'app/views/pages/about.html.liquid': '

About

', + }); + + const source = + '{% if true %}{% liquid\n assign url = "/about"\n%}About{% endif %}'; + documentManager.open('file:///project/app/views/pages/home.html.liquid', source, 1); + + const urlOffset = source.indexOf('{{ url }}'); + const urlLine = source.slice(0, urlOffset).split('\n').length - 1; + const urlChar = urlOffset - source.lastIndexOf('\n', urlOffset - 1) - 1; + const params: DefinitionParams = { + textDocument: { uri: 'file:///project/app/views/pages/home.html.liquid' }, + position: { line: urlLine, character: urlChar + 3 }, // Inside {{ url }} + }; + + const result = await provider.definitions(params); + assert(result); + expect(result).toHaveLength(1); + expect(result[0].targetUri).toBe('file:///project/app/views/pages/about.html.liquid'); + }); }); describe('format-aware go-to-definition', () => { diff --git a/packages/platformos-language-server-common/src/server/startServer.spec.ts b/packages/platformos-language-server-common/src/server/startServer.spec.ts index f0d2518..08f172b 100644 --- a/packages/platformos-language-server-common/src/server/startServer.spec.ts +++ b/packages/platformos-language-server-common/src/server/startServer.spec.ts @@ -7,11 +7,13 @@ import { DidRenameFilesNotification, FileChangeType, PublishDiagnosticsNotification, + DefinitionRequest, } from 'vscode-languageserver'; import { MockConnection, mockConnection } from '../test/MockConnection'; import { Dependencies } from '../types'; import { CHECK_ON_CHANGE, CHECK_ON_OPEN, CHECK_ON_SAVE } from './Configuration'; import { startServer } from './startServer'; +import { SearchPathsLoader } from '../utils/searchPaths'; const sleep = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); @@ -76,6 +78,7 @@ describe('Module: server', () => { afterEach(() => { vi.useRealTimers(); + vi.restoreAllMocks(); }); it("should log Let's roll! on successful setup", async () => { @@ -315,6 +318,65 @@ describe('Module: server', () => { ); }); + it('go-to-definition reflects updated search paths after saving app/config.yml', async () => { + // Setup file tree: config pointing to theme/dress, both dress and simple partials present + fileTree['app/config.yml'] = 'theme_search_paths:\n - theme/dress'; + fileTree['app/views/partials/theme/dress/card.liquid'] = 'dress card'; + fileTree['app/views/partials/theme/simple/card.liquid'] = 'simple card'; + + connection.setup(); + await flushAsync(); + + // Open a document referencing the partial + const source = "{% theme_render_rc 'card' %}"; + connection.openDocument(filePath, source); + await flushAsync(); + + // Request definition — character 21 is inside 'card' + const params = { + textDocument: { uri: fileURI }, + position: { line: 0, character: 21 }, + }; + const result1 = (await connection.triggerRequest(DefinitionRequest.method, params)) as any[]; + expect(result1).toHaveLength(1); + expect(result1[0].targetUri).toContain('theme/dress/card.liquid'); + + // Mutate config to point to theme/simple, then save the config file + fileTree['app/config.yml'] = 'theme_search_paths:\n - theme/simple'; + connection.saveDocument('app/config.yml'); + await flushAsync(); + + // Definition should now resolve to theme/simple + const result2 = (await connection.triggerRequest(DefinitionRequest.method, params)) as any[]; + expect(result2).toHaveLength(1); + expect(result2[0].targetUri).toContain('theme/simple/card.liquid'); + }); + + it('should invalidate search-paths cache immediately when app/config.yml is saved', async () => { + connection.setup(); + await flushAsync(); + + const invalidateSpy = vi.spyOn(SearchPathsLoader.prototype, 'invalidate'); + + // Saving app/config.yml should immediately invalidate the cache + connection.saveDocument('app/config.yml'); + await flushAsync(); + + expect(invalidateSpy).toHaveBeenCalledOnce(); + }); + + it('should NOT invalidate search-paths cache when an unrelated file is saved', async () => { + connection.setup(); + await flushAsync(); + + const invalidateSpy = vi.spyOn(SearchPathsLoader.prototype, 'invalidate'); + + connection.saveDocument('app/views/partials/code.liquid'); + await flushAsync(); + + expect(invalidateSpy).not.toHaveBeenCalled(); + }); + it('should trigger a re-check on did delete files notifications', async () => { connection.setup(); await flushAsync(); diff --git a/packages/platformos-language-server-common/src/test/MockConnection.ts b/packages/platformos-language-server-common/src/test/MockConnection.ts index df3fecc..99292e4 100644 --- a/packages/platformos-language-server-common/src/test/MockConnection.ts +++ b/packages/platformos-language-server-common/src/test/MockConnection.ts @@ -2,6 +2,7 @@ import { vi } from 'vitest'; import { EventEmitter } from 'node:events'; import { createConnection } from 'vscode-languageserver/lib/common/server'; import { + CancellationToken, ClientCapabilities, DidChangeTextDocumentNotification, DidCloseTextDocumentNotification, @@ -47,7 +48,11 @@ type MockConnectionMethods = { */ export type MockConnection = ReturnType & MockConnectionMethods; -function protocolConnection(requests: EventEmitter, notifications: EventEmitter) { +function protocolConnection( + requests: EventEmitter, + notifications: EventEmitter, + requestHandlers: Map, +) { return { dispose: vi.fn(), end: vi.fn(), @@ -61,12 +66,13 @@ function protocolConnection(requests: EventEmitter, notifications: EventEmitter) notifications.addListener(type.method, handler); }), onRequest: vi.fn().mockImplementation((type: MessageSignature, handler) => { + requestHandlers.set(type.method, handler); requests.addListener(type.method, handler); }), onUnhandledNotification: vi.fn(), sendNotification: vi.fn().mockReturnValue(Promise.resolve()), sendProgress: vi.fn(), - sendRequest: vi.fn(), + sendRequest: vi.fn().mockReturnValue(Promise.resolve()), trace: vi.fn().mockReturnValue(Promise.resolve()), } satisfies ProtocolConnection; } @@ -80,7 +86,8 @@ export function mockConnection(rootUri: string): MockConnection { const requests = new EventEmitter(); const notifications = new EventEmitter(); - const spies = protocolConnection(requests, notifications); + const requestHandlers = new Map(); + const spies = protocolConnection(requests, notifications, requestHandlers); // Create a real "connection" with the fake communication channel const connection = createConnection(() => spies, watchDog); @@ -92,10 +99,15 @@ export function mockConnection(rootUri: string): MockConnection { notifications.emit(method, params); }; - // Create a mock way to trigger requests in our tests + // Create a mock way to trigger requests in our tests and get the response back. + // Calls the registered handler directly so that the return value is available. const triggerRequest: MockConnection['triggerRequest'] = async (...args: any[]) => { const [type, params] = args; const method = typeof type === 'string' ? type : type.method; + const handler = requestHandlers.get(method); + if (handler) { + return handler(params, CancellationToken.None); + } requests.emit(method, params); };