Skip to content

Commit

Permalink
Add unit tests + changeset
Browse files Browse the repository at this point in the history
  • Loading branch information
charlespwd committed Mar 11, 2024
1 parent 78ecfd6 commit c465226
Show file tree
Hide file tree
Showing 8 changed files with 114 additions and 55 deletions.
5 changes: 5 additions & 0 deletions .changeset/curly-bikes-confess.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@shopify/theme-check-node': patch
---

Fix theme file glob on Windows (CLI issue)
2 changes: 0 additions & 2 deletions packages/theme-check-common/src/path.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
import { RelativePath, AbsolutePath } from './types';

export function relative(absolutePath: AbsolutePath, root: AbsolutePath): RelativePath {
// TODO (#15): do the right thing for windows and shit.
return absolutePath.replace(root, '').replace(/^\//, '');
}

export function join(...paths: string[]): string {
// TODO (#15): do the right thing, collapse slashes and shit.
return paths.map(removeTrailingSlash).join('/');
}

Expand Down
10 changes: 8 additions & 2 deletions packages/theme-check-common/src/to-source-code.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,19 +40,25 @@ export function toSourceCode(

if (isLiquid) {
return {
absolutePath,
absolutePath: normalize(absolutePath),
source,
type: SourceCodeType.LiquidHtml,
ast: parseLiquid(source),
version,
};
} else {
return {
absolutePath,
absolutePath: normalize(absolutePath),
source,
type: SourceCodeType.JSON,
ast: parseJSON(source),
version,
};
}
}

type MaybeWindowsPath = string;

function normalize(path: MaybeWindowsPath): string {
return path.replace(/\\/g, '/');
}
14 changes: 11 additions & 3 deletions packages/theme-check-common/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,16 @@ export type Theme = SourceCode<SourceCodeType>[];

export type SourceCode<T = SourceCodeType> = T extends SourceCodeType
? {
absolutePath: string; // /path/to/snippet/foo.liquid
/** A normalized absolute path to the file. Assumes forwards slashes. */
absolutePath: string;
/** The type is used as a discriminant for type narrowing */
type: T;
/** The version is used by the Language Server to make sure the Client and Server are in sync */
version?: number;
/** The contents of the file */
source: string;
type: T; // Liquid | LiquidHtml | JSON
ast: AST[T] | Error; // LiquidAST | LiquidHtmlAST | JSON object | null when unparseable
/** The AST representation of the file, or an Error instance when the file is unparseable */
ast: AST[T] | Error;
}
: never;

Expand Down Expand Up @@ -74,7 +79,10 @@ export type NodeTypes = {
}[T];
};

/** Assumes forward slashes for simplicity internally */
export type AbsolutePath = string;

/** Assumes forward slashes for simplicity internally */
export type RelativePath = string;

export type ChecksSettings = {
Expand Down
44 changes: 2 additions & 42 deletions packages/theme-check-node/src/find-root.spec.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import { describe, it, expect, beforeEach, afterEach, beforeAll, afterAll } from 'vitest';
import { findRoot } from './find-root';
import * as path from 'node:path';
import * as fs from 'node:fs/promises';
import * as mktemp from 'mktemp';
import { Workspace } from './test/test-helpers';
import { makeTempWorkspace } from './test/test-helpers';

const theme = {
locales: {
Expand Down Expand Up @@ -120,42 +119,3 @@ describe('Unit: findRoot', () => {
expect(root).toBe(workspace.path('taeRootThemeCheckYML'));
});
});

type Tree = {
[k in string]: Tree | string;
};

interface Workspace {
root: string;
path(relativePath: string): string;
clean(): Promise<any>;
}

async function makeTempWorkspace(structure: Tree): Promise<Workspace> {
const root = await mktemp.createDir(path.join(__dirname, '..', '.XXXXX'));
if (!root) throw new Error('Could not create temp dir for temp workspace');

await createFiles(structure, [root]);

return {
root,
path: (relativePath) => path.join(root, ...relativePath.split('/')),
clean: async () => fs.rm(root, { recursive: true, force: true }),
};

function createFiles(tree: Tree, ancestors: string[]): Promise<any> {
const promises: Promise<any>[] = [];
for (const [pathEl, value] of Object.entries(tree)) {
if (typeof value === 'string') {
promises.push(fs.writeFile(path.join(...ancestors, pathEl), value, 'utf8'));
} else {
promises.push(
fs
.mkdir(path.join(...ancestors, pathEl))
.then(() => createFiles(value, ancestors.concat(pathEl))),
);
}
}
return Promise.all(promises);
}
}
41 changes: 41 additions & 0 deletions packages/theme-check-node/src/index.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import { describe, it, expect, assert, beforeEach, afterEach } from 'vitest';
import * as path from 'node:path';
import { Config, SourceCodeType, getTheme } from './index';
import { Workspace, makeTempWorkspace } from './test/test-helpers';

describe('Unit: getTheme', () => {
let workspace: Workspace;

beforeEach(async () => {
workspace = await makeTempWorkspace({
locales: {
'en.default.json': '{}',
},
snippets: {
'header.liquid': '',
},
});
});

afterEach(async () => {
await workspace.clean();
});

it('should correctly get theme on all platforms', async () => {
const config: Config = {
checks: [],
root: workspace.root,
settings: {},
};

const theme = await getTheme(config);
expect(theme).to.have.lengthOf(2);
const jsonFile = theme.find((sc) => sc.type === SourceCodeType.JSON);
assert(jsonFile);

// internally we expect the path to be normalized
expect(jsonFile.absolutePath).to.equal(
workspace.path('locales/en.default.json').replace(/\\/g, '/'),
);
});
});
11 changes: 6 additions & 5 deletions packages/theme-check-node/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ export async function themeCheckRun(root: string, configPath?: string): Promise<
if (!defaultTranslationsFile) {
return defaultLocale;
}
// assumes the path is normalized and '/' are used as separators
const defaultTranslationsFileLocale = defaultTranslationsFile.absolutePath.match(
/locales\/(.*)\.default\.json$/,
)?.[1];
Expand Down Expand Up @@ -102,15 +103,15 @@ async function getThemeAndConfig(
}

export async function getTheme(config: Config): Promise<Theme> {

// On windows machines - the separator provided by path.join is '\'
// On windows machines - the separator provided by path.join is '\'
// however the glob function fails silently since '\' is used to escape glob charater
// as mentioned in the documentation of node-glob

// the path is normalised and '\' are replaced with '/' and then passed to the glob function
const normalized_path = path.normalize(path.join(config.root, '**/*.{liquid,json}')).replace(/\\/g, '/');

const paths = await asyncGlob(normalized_path).then((result) =>
const normalizedGlob = path
.normalize(path.join(config.root, '**/*.{liquid,json}'))
.replace(/\\/g, '/');
const paths = await asyncGlob(normalizedGlob).then((result) =>
// Global ignored paths should not be part of the theme
result.filter((filePath) => !isIgnored(filePath, config)),
);
Expand Down
42 changes: 41 additions & 1 deletion packages/theme-check-node/src/test/test-helpers.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import * as mktemp from 'mktemp';
import fs from 'node:fs/promises';
import path from 'node:path';
import os from 'node:os';
import path from 'node:path';

export async function makeTmpFolder() {
const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'test-'));
Expand Down Expand Up @@ -61,3 +62,42 @@ export async function createMockNodeModule(
await fs.writeFile(path.join(nodeModuleRoot, 'index.js'), moduleContent);
return nodeModuleRoot;
}

export type Tree = {
[k in string]: Tree | string;
};

export interface Workspace {
root: string;
path(relativePath: string): string;
clean(): Promise<any>;
}

export async function makeTempWorkspace(structure: Tree): Promise<Workspace> {
const root = await mktemp.createDir(path.join(__dirname, '..', '.XXXXX'));
if (!root) throw new Error('Could not create temp dir for temp workspace');

await createFiles(structure, [root]);

return {
root,
path: (relativePath) => path.join(root, ...relativePath.split('/')),
clean: async () => fs.rm(root, { recursive: true, force: true }),
};

function createFiles(tree: Tree, ancestors: string[]): Promise<any> {
const promises: Promise<any>[] = [];
for (const [pathEl, value] of Object.entries(tree)) {
if (typeof value === 'string') {
promises.push(fs.writeFile(path.join(...ancestors, pathEl), value, 'utf8'));
} else {
promises.push(
fs
.mkdir(path.join(...ancestors, pathEl))
.then(() => createFiles(value, ancestors.concat(pathEl))),
);
}
}
return Promise.all(promises);
}
}

0 comments on commit c465226

Please sign in to comment.