Skip to content

Commit

Permalink
test(cli): make implicit mocks explicit (#3318)
Browse files Browse the repository at this point in the history
This makes it easier to migrate to Node test runner in the future. We
cannot migrate today because we still need module mocking, which will
become available in Node 22.
  • Loading branch information
tido64 authored Aug 29, 2024
1 parent 13e428e commit 66ba538
Show file tree
Hide file tree
Showing 28 changed files with 355 additions and 308 deletions.
2 changes: 2 additions & 0 deletions .changeset/few-starfishes-give.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
---
---
4 changes: 2 additions & 2 deletions packages/cli/src/bundle/kit-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,9 @@ function getDefaultBundleParameters(platform: string) {
*/
export function getCliPlatformBundleConfigs(
id?: string,
overridePlatform?: AllPlatforms
overridePlatform?: AllPlatforms,
kitConfig = getKitConfig()
): CliPlatformBundleConfig[] {
const kitConfig = getKitConfig();
const maybeBundleConfig = kitConfig
? getBundleConfig(kitConfig, id)
: undefined;
Expand Down
10 changes: 6 additions & 4 deletions packages/cli/src/bundle/metro.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { info } from "@rnx-kit/console";
import type { BundleArgs as MetroBundleArgs } from "@rnx-kit/metro-service";
import { bundle } from "@rnx-kit/metro-service";
import type { ConfigT } from "metro-config";
import * as nodefs from "node:fs";
import * as path from "node:path";
import { ensureDir } from "../helpers/filesystem";
import { customizeMetroConfig } from "../helpers/metro-config";
Expand All @@ -24,7 +25,8 @@ export async function metroBundle(
bundleConfig: CliPlatformBundleConfig,
dev: boolean,
minify?: boolean,
output = bundle
output = bundle,
fs = nodefs
): Promise<void> {
info(`Bundling ${bundleConfig.platform}...`);

Expand All @@ -49,12 +51,12 @@ export async function metroBundle(
};

// ensure all output directories exist
ensureDir(path.dirname(metroBundleArgs.bundleOutput));
ensureDir(path.dirname(metroBundleArgs.bundleOutput), fs);
if (metroBundleArgs.sourcemapOutput) {
ensureDir(path.dirname(metroBundleArgs.sourcemapOutput));
ensureDir(path.dirname(metroBundleArgs.sourcemapOutput), fs);
}
if (metroBundleArgs.assetsDest) {
ensureDir(metroBundleArgs.assetsDest);
ensureDir(metroBundleArgs.assetsDest, fs);
}

// create the bundle
Expand Down
76 changes: 43 additions & 33 deletions packages/cli/src/copy-assets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,11 @@ import {
import { findUp } from "@rnx-kit/tools-node/path";
import type { AllPlatforms } from "@rnx-kit/tools-react-native";
import { parsePlatform } from "@rnx-kit/tools-react-native";
import type { SpawnSyncOptions } from "child_process";
import { spawnSync } from "child_process";
import * as fs from "fs";
import * as nodefs from "fs";
import * as os from "os";
import * as path from "path";
import type { SpawnSyncOptions } from "node:child_process";
import { spawnSync } from "node:child_process";
import * as nodefs from "node:fs";
import * as os from "node:os";
import * as path from "node:path";
import { ensureDir } from "./helpers/filesystem";

export type AndroidArchive = {
Expand Down Expand Up @@ -71,7 +70,7 @@ const defaultAndroidConfig: Required<Required<AndroidArchive>["android"]> = {
kotlinVersion: "1.7.22",
};

function cloneFile(src: string, dest: string) {
function cloneFile(src: string, dest: string, fs = nodefs) {
return fs.promises.copyFile(src, dest, fs.constants.COPYFILE_FICLONE);
}

Expand All @@ -86,7 +85,10 @@ function ensureOption(options: Options, opt: string, flag = opt) {
}
}

function findGradleProject(projectRoot: string): string | undefined {
function findGradleProject(
projectRoot: string,
fs = nodefs
): string | undefined {
if (fs.existsSync(path.join(projectRoot, "android", "build.gradle"))) {
return path.join(projectRoot, "android");
}
Expand All @@ -106,28 +108,29 @@ function isAssetsConfig(config: unknown): config is AssetsConfig {
return typeof config === "object" && config !== null && "getAssets" in config;
}

export function versionOf(pkgName: string): string {
const packageDir = findPackageDependencyDir(pkgName);
export function versionOf(pkgName: string, fs = nodefs): string {
const packageDir = findPackageDependencyDir(pkgName, undefined, fs);
if (!packageDir) {
throw new Error(`Could not find module '${pkgName}'`);
}

const { version } = readPackage(packageDir);
const { version } = readPackage(packageDir, fs);
return version;
}

function getAndroidPaths(
context: Context,
packageName: string,
{ targetName, version, output }: AndroidArchive
{ targetName, version, output }: AndroidArchive,
fs = nodefs
) {
const projectRoot = findPackageDependencyDir(packageName);
const projectRoot = findPackageDependencyDir(packageName, undefined, fs);
if (!projectRoot) {
throw new Error(`Could not find module '${packageName}'`);
}

const gradleFriendlyName = targetName || gradleTargetName(packageName);
const aarVersion = version || versionOf(packageName);
const aarVersion = version || versionOf(packageName, fs);

switch (packageName) {
case "hermes-engine":
Expand All @@ -139,7 +142,7 @@ function getAndroidPaths(
destination: path.join(
context.options.assetsDest,
"aar",
`hermes-release-${versionOf(packageName)}.aar`
`hermes-release-${versionOf(packageName, fs)}.aar`
),
};

Expand All @@ -157,7 +160,7 @@ function getAndroidPaths(
};

default: {
const androidProject = findGradleProject(projectRoot);
const androidProject = findGradleProject(projectRoot, fs);
return {
targetName: gradleFriendlyName,
version: aarVersion,
Expand Down Expand Up @@ -193,14 +196,15 @@ function run(command: string, args: string[], options: SpawnSyncOptions) {
export async function assembleAarBundle(
context: Context,
packageName: string,
{ aar }: NativeAssets
{ aar }: NativeAssets,
fs = nodefs
): Promise<void> {
if (!aar) {
return;
}

const wrapper = os.platform() === "win32" ? "gradlew.bat" : "gradlew";
const gradlew = findUp(wrapper);
const gradlew = findUp(wrapper, undefined, fs);
if (!gradlew) {
warn(`Skipped \`${packageName}\`: cannot find \`${wrapper}\``);
return;
Expand All @@ -209,7 +213,8 @@ export async function assembleAarBundle(
const { targetName, version, androidProject, output } = getAndroidPaths(
context,
packageName,
aar
aar,
fs
);
if (!androidProject || !output) {
warn(`Skipped \`${packageName}\`: cannot find \`build.gradle\``);
Expand All @@ -219,7 +224,7 @@ export async function assembleAarBundle(
const { env: customEnv, dependencies, android } = aar;
const env = {
NODE_MODULES_PATH: path.join(process.cwd(), "node_modules"),
REACT_NATIVE_VERSION: versionOf("react-native"),
REACT_NATIVE_VERSION: versionOf("react-native", fs),
...process.env,
...customEnv,
};
Expand All @@ -239,7 +244,8 @@ export async function assembleAarBundle(
const { targetName, output, destination } = getAndroidPaths(
context,
dependencyName,
aar
aar,
fs
);
if (output) {
if (!fs.existsSync(output)) {
Expand Down Expand Up @@ -336,7 +342,9 @@ export async function assembleAarBundle(
run(gradlew, targets, { cwd: buildDir, stdio: "inherit", env });
}

await Promise.all(targetsToCopy.map(([src, dest]) => cloneFile(src, dest)));
await Promise.all(
targetsToCopy.map(([src, dest]) => cloneFile(src, dest, fs))
);
}

function copyFiles(
Expand Down Expand Up @@ -372,10 +380,10 @@ export async function copyAssets(
await Promise.all(tasks);
}

export async function gatherConfigs({
projectRoot,
manifest,
}: Context): Promise<Record<string, AssetsConfig | null> | undefined> {
export async function gatherConfigs(
{ projectRoot, manifest }: Context,
fs = nodefs
): Promise<Record<string, AssetsConfig | null> | undefined> {
const { dependencies, devDependencies } = manifest;
const packages = [...keysOf(dependencies), ...keysOf(devDependencies)];
if (packages.length === 0) {
Expand Down Expand Up @@ -472,11 +480,12 @@ export async function gatherConfigs({
*/
export async function copyProjectAssets(
options: Options,
{ root: projectRoot, reactNativePath }: CLIConfig
{ root: projectRoot, reactNativePath }: CLIConfig,
fs = nodefs
): Promise<void> {
const manifest = readPackage(projectRoot);
const context = { projectRoot, manifest, options, reactNativePath };
const assetConfigs = await gatherConfigs(context);
const assetConfigs = await gatherConfigs(context, fs);
if (!assetConfigs) {
return;
}
Expand All @@ -496,10 +505,10 @@ export async function copyProjectAssets(
const assets = await getAssets(context);
if (options.bundleAar && assets.aar) {
info(`Assembling "${packageName}"`);
await assembleAarBundle(context, packageName, assets);
await assembleAarBundle(context, packageName, assets, fs);
} else {
info(`Copying assets for "${packageName}"`);
await copyAssets(context, packageName, assets);
await copyAssets(context, packageName, assets, fs);
}
}

Expand All @@ -510,14 +519,15 @@ export async function copyProjectAssets(
const { output, destination } = getAndroidPaths(
context,
dependencyName,
dummyAar
dummyAar,
fs
);
if (
output &&
(!fs.existsSync(destination) || fs.statSync(destination).isDirectory())
) {
info(`Copying Android Archive of "${dependencyName}"`);
copyTasks.push(cloneFile(output, destination));
copyTasks.push(cloneFile(output, destination, fs));
}
}
await Promise.all(copyTasks);
Expand All @@ -531,7 +541,7 @@ export const rnxCopyAssetsCommand = {
func: (_argv: string[], config: CLIConfig, options: Options) => {
ensureOption(options, "platform");
ensureOption(options, "assetsDest", "assets-dest");
return copyProjectAssets(options, config);
return copyProjectAssets(options, config, nodefs);
},
options: [
{
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/helpers/filesystem.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import * as nodefs from "fs"; // Cannot use `node:fs` because of Jest mocks
import * as nodefs from "node:fs";

export function ensureDir(p: string, fs = nodefs): void {
fs.mkdirSync(p, { recursive: true, mode: 0o755 });
Expand Down
5 changes: 0 additions & 5 deletions packages/cli/test/__mocks__/child_process.js

This file was deleted.

1 change: 0 additions & 1 deletion packages/cli/test/__mocks__/fs.js

This file was deleted.

6 changes: 3 additions & 3 deletions packages/cli/test/bin/context.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ jest.mock("@rnx-kit/tools-react-native/context", () => ({
},
}));

describe("getCoreCommands()", () => {
describe("bin/context/getCoreCommands()", () => {
it("strips `rnx-` prefix from all commands", () => {
const coreCommands = getCoreCommands();
for (let i = 0; i < coreCommands.length; ++i) {
Expand All @@ -24,7 +24,7 @@ describe("getCoreCommands()", () => {
});
});

describe("uniquify()", () => {
describe("bin/context/uniquify()", () => {
function makeCommand(name: string, description: string): Command<false> {
return { name, description } as Command<false>;
}
Expand All @@ -48,7 +48,7 @@ describe("uniquify()", () => {
});
});

describe("loadContext()", () => {
describe("bin/context/loadContext()", () => {
afterAll(() => {
jest.resetAllMocks();
});
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/test/bin/externalCommands.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ function mockContext(context: unknown = {}): Config {
return context as Config;
}

describe("findExternalCommands()", () => {
describe("bin/externalCommands/findExternalCommands()", () => {
afterAll(() => {
jest.resetAllMocks();
});
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/test/bundle.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { rnxBundleCommand } from "../src/bundle";
import { asBoolean, asNumber, asStringArray } from "../src/helpers/parsers";

describe("rnx-clean", () => {
describe("rnx-bundle", () => {
it("correctly parses cli arguments", () => {
for (const { name, parse } of rnxBundleCommand.options) {
if (name.endsWith("[boolean]")) {
Expand Down
14 changes: 0 additions & 14 deletions packages/cli/test/bundle/__mocks__/@rnx-kit/config.js

This file was deleted.

1 change: 0 additions & 1 deletion packages/cli/test/bundle/__mocks__/@rnx-kit/console.js

This file was deleted.

This file was deleted.

Loading

0 comments on commit 66ba538

Please sign in to comment.