Skip to content
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.

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

test(cli): make implicit mocks explicit #3318

Merged
merged 2 commits into from
Aug 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
Loading