Use memfs for commons by ShinobuTakahashi · Pull Request #1576 · akashic-games/akashic-cli

ShinobuTakahashi · 2025-07-30T01:31:05Z

概要

#1561 の browserify -> rollup 変更に伴うテスト周りの修正。

akashic-cli-commons の mock-fs を memfs へ変更。
memfs で対応できない箇所は、実ファイルでテストを実施。

#1574 (comment) でいただいたコメントを反映し、実ファイルを展開するユーティリティを追加。

備考
#1561 へマージします。commons 以外の対応が完了しCIが通るまでマージしません ローカルで commonsのテストが通る事は確認しています。
scan と lib-manage で一部 commons の NodeModules.ts を利用している箇所でテストが落ちている。別途対応(#1586)。

ShinobuTakahashi · 2025-07-30T01:37:38Z

packages/akashic-cli-commons/src/TestUtil.ts

@@ -0,0 +1,48 @@
+import * as fs from "fs";


他パッケージのテストでも使うので "TestUtil" としましたが、ファイル名に Test と入ってるのが違和感があり、"FsContentUtil" とかのファイル名の方が良いでしょうか？

ファイル名はそのままでいいと思うのですが、モジュール本体に関係ないので __tests__/ 以下におきませんか。

他パッケージから __tests__/ 以下を import するのも変ではありますが、これは単に (テスト用ユーティリティのパッケージを分離する) 手間を省いただけなので。テストの手間を省く目的のために本体に影響が出るのは避けたいです。

他パッケージで commons の __tests__ 以下を参照する箇所では (本当はテスト用ユーティリティとして獨立させるべきところ手間を省いて commons のコードを参照している旨) コメントをつけておきたいです。

ありがとうございます。 __tests__/helper/ 配下に移動しました。

ShinobuTakahashi · 2025-07-30T01:38:53Z

packages/akashic-cli-commons/src/__tests__/NodeModulesSpec.ts

-			mockfs(mockFsContent);
-
-			let pkgjsonPaths = await NodeModules.listModuleFiles(".", ["dummy@1", "dummy2", "dummy3"], logger);
+			let pkgjsonPaths = await NodeModules.listModuleFiles(targetPath, ["dummy", "dummy2", "dummy3"], logger);


"dummy@1" は rollup では解決できないので "dummy" へ修正

xnv · 2025-07-30T05:49:50Z

packages/akashic-cli-commons/package.json

  "devDependencies": {
    "@akashic/eslint-config": "^3.0.1",
-    "@types/mock-fs": "4.13.4",
+    "@types/mem-fs": " 2.2.0",


memfs と mem-fs は別物では。memfs は @types 不要ではないかと思います。

ありがとうございます。削除しました。

xnv · 2025-07-30T05:55:38Z

packages/akashic-cli-commons/src/TestUtil.ts

@@ -0,0 +1,48 @@
+import * as fs from "fs";


ファイル名はそのままでいいと思うのですが、モジュール本体に関係ないので __tests__/ 以下におきませんか。

他パッケージから __tests__/ 以下を import するのも変ではありますが、これは単に (テスト用ユーティリティのパッケージを分離する) 手間を省いただけなので。テストの手間を省く目的のために本体に影響が出るのは避けたいです。

他パッケージで commons の __tests__ 以下を参照する箇所では (本当はテスト用ユーティリティとして獨立させるべきところ手間を省いて commons のコードを参照している旨) コメントをつけておきたいです。

xnv · 2025-07-30T05:57:11Z

packages/akashic-cli-commons/src/__tests__/FileSystemSpec.ts

+  return memfs.fs;
+});
+
+// editorconfig の parse() で memfs でモックした .editorconfig が読み込めないため parse() をモックし


コメントが途中で途切れていませんか。

ありがとうございます。修正しました。

xnv · 2025-07-30T05:58:35Z

packages/akashic-cli-commons/src/TestUtil.ts

+	setupFsContent(dir, def);
+	return {
+		"path": dir,
+		func: () => {


名前をつけて外部から呼ぶものを func と呼ぶのはさすがに変に思います。 dispose() とかじゃないんでしょうか。

確かに仰る通りです。dispose へ修正しました。

xnv · 2025-07-30T05:58:46Z

packages/akashic-cli-commons/src/TestUtil.ts

+	const dir = fs.mkdtempSync(target);
+	setupFsContent(dir, def);
+	return {
+		"path": dir,


" 不要そうです。

修正しました。

xnv · 2025-08-01T06:03:46Z

packages/akashic-cli-commons/src/__tests__/NodeModulesSpec.ts

 		}
 	};

+	const baseDir = path.resolve(__dirname, "..", "__tests__", "output");


これ mkdtemp(path.resolve(__dirname, "..", "__tests__", "fixture-")) とはできないでしょうか。

あまりないと思いますが、並列にテストすると崩壊するので原則的にはディレクトリを固定にしたくないように思います。またそうでなくても __tests__/output/ というは普通にも使いかねない名前だと思えるので、被りそうにない名前にはしておきたいです。

fixture- に修正しました。TestUtil の方で mkdtemp() で fixture-xxxxx のディレクトリが作成される事を確認しました。

xnv · 2025-08-01T06:17:02Z

packages/akashic-cli-commons/src/__tests__/NodeModulesSpec.ts


+	const baseDir = path.resolve(__dirname, "..", "__tests__", "output");
 	let logger: Logger;
+	let fsContentResult: testUtil.FsContentResult;


原則的に変数は「型名」でなく「意味」で名付けたいです。(たとえば (敵の攻撃力 - 自分の防御力) という計算の結果は、型名にしたがって num と呼ぶより意味で damage と呼ぶ方がわかりやすいように思います)

これはテスト用の fixture のファイルなので、たとえば fixtureContents とするのはどうでしょうか。

ありがとうございます。修正しました。

xnv · 2025-08-01T06:18:52Z

packages/akashic-cli-commons/src/__tests__/NodeModulesSpec.ts

+	const baseDir = path.resolve(__dirname, "..", "__tests__", "output");
 	let logger: Logger;
+	let fsContentResult: testUtil.FsContentResult;
+	let targetPath = "";


"target" という新たな言葉を導入されていますが、上に合わせるなら fixtureContentsPath になるかと思います。
利用箇所も多くないので変数を作らず都度 fixtureContents.path としてもよさそうです。

fixtureContents.path を利用するよう修正しました。

xnv · 2025-08-01T07:02:45Z

packages/akashic-cli-commons/src/__tests__/NodeModulesSpec.ts

+			const pkgJsonKey3 = path.resolve(targetPath, "node_modules/dummy3/package.json");
+
+			const expectObj: { [key: string]: string } = {};
+			expectObj[pkgJsonKey1] =  path.resolve(targetPath,"node_modules/dummy/main.js").replace(/^\//, "");


空白が 2 つになっています＋ , の後の空白が抜けています。他の行にもあるので見直しをお願いします。

ありがとうございます。修正しました。

xnv · 2025-08-01T07:04:16Z

packages/akashic-cli-commons/src/__tests__/NodeModulesSpec.ts

+			let packageJsonFiles = NodeModules.listPackageJsonsFromScriptsPath(targetPath, filePaths);
+			// 本来はルート直下の ./node_modules のパスだが、テストで node_modules のパスがルート直下ではないためパスを生成
+			packageJsonFiles = packageJsonFiles.map(p => path.resolve(targetPath, p));
+			const moduleMainPaths = NodeModules.listModuleMainPaths(packageJsonFiles);


moduleMainScripts() のテストが moduleMainPaths() のテストに変わっていますが意図されたものでしょうか。前者が残っているなら前者も引き続きテストすべきに思えます。

すみません、ミスです。元の moduleMainScripts() に修正しました。

xnv · 2025-08-01T07:07:23Z

packages/akashic-cli-commons/src/__tests__/NodeModulesSpec.ts

+			const pkgJsonKey1 = path.resolve(targetPath,"node_modules/dummy/package.json");
+			const gameJsonKey = path.resolve(targetPath, "node_modules/dummy/node_modules/dummyChild/package.json");
+			const pkgJsonKey3 = path.resolve(targetPath, "node_modules/dummy3/package.json");
+
+			const expectObj: { [key: string]: string } = {};
+			expectObj[pkgJsonKey1] =  path.resolve(targetPath,"node_modules/dummy/main.js").replace(/^\//, "");
+			expectObj[gameJsonKey] =  path.resolve(targetPath, "node_modules/dummy/node_modules/dummyChild/main.js").replace(/^\//, "");
+			expectObj[pkgJsonKey3] =  path.resolve(targetPath, "node_modules/dummy3/index.js").replace(/^\//, "");


expectObj とか gameJsonKey とか名前をつけなきゃいけないの面倒そうですが、以下のように直接書けたりしないでしょうか。

expect(...).toEqual({ [path.resolve(targetPath,"node_modules/dummy/package.json")]: path.resolve(targetPath,"node_modules/dummy/main.js").replace(/^\//, ""), [path.resolve(targetPath, "node_modules/dummy/node_modules/dummyChild/package.json")]: path.resolve(targetPath, "node_modules/dummy/node_modules/dummyChild/main.js").replace(/^\//, ""), ... });

ありがとうございます。ご指摘いただいたよう直接書くよう修正しました。

xnv · 2025-08-01T07:17:32Z

packages/akashic-cli-commons/src/__tests__/helpers/TestUtil.ts

+	}
+}
+
+export function preperFsContent(def: FSContentDescDir, baseDir?: string): FsContentResult  {


prepare の typo に見えます。

実際省略するケースがないなら baseDir も必須にしてしまいませんか。(テスト用ユーティリティで汎用の便利挙動をつけておく動機が薄い)

ありがとうございます。typo、baseDir を必須化に修正しました。

xnv · 2025-08-01T07:21:15Z

packages/akashic-cli-commons/src/__tests__/helpers/TestUtil.ts

+	[filename: string]: FSContentDescDir | string;
+}
+
+export interface FsContentResult {


〜Result は「〜の結果」に無理やり名前をつけた感じなので、名付けるなら「〜」の部分は関数名にしたいかなと思います。( FsContentResult だと「FsContent した結果」みたいな意味に感じられて違和感があります)

逆にすぱっと FsContent と言い切るという手もありますが、それにしては持ってるプロパティが簡素すぎるので、PrepareFsContentResult ですかね。

ありがとうございます。最初に FsContent にしたのですが、仰る通りプロパティが少なく Content なのかと疑問に思ったので Result を付けた経緯です。
PrepareFsContentResult に修正しました。

xnv · 2025-08-01T07:22:21Z

packages/akashic-cli-commons/src/__tests__/helpers/TestUtil.ts

+import * as os from "os";
+import path from "path";
+
+interface FSContentDescDir {


全体的に私がざっと例に出したてきとうな名前なのでいまいちな気はしてるんですが、特にこれは「変数名で def を使ってるのでそれに合わせたい」のと「FS の表記を揃えたい」ので FsContentDefinition ですかね。(逆に変数名を desc にすることもできると思います)

変数名そのままで、 interface 名を FsContentDefinition に修正しました。

kamakiri01 · 2025-08-05T04:41:52Z

packages/akashic-cli-commons/src/__tests__/FileSystemSpec.ts

 		await expect(readJSON("./foo/test.json")).rejects.toMatchObject({ code: "ENOENT" });
 	});
-});
+});


いくつかのファイルで末尾改行がなくなっていそうです。

ありがとうございます。修正しました。

Use memfs other

ShinobuTakahashi added 2 commits July 29, 2025 14:04

Use memfs, remove mock-fs

bb82fe8

Fix spec

7386235

ShinobuTakahashi changed the base branch from main to remove-browserify-use-rollup July 30, 2025 01:33

ShinobuTakahashi commented Jul 30, 2025

View reviewed changes

ShinobuTakahashi requested review from kamakiri01 and xnv July 30, 2025 01:56

xnv reviewed Jul 30, 2025

View reviewed changes

Fix TestUtil.ts,

afe3253

xnv reviewed Aug 1, 2025

View reviewed changes

Fix NodeModulesSpec, TestUtil

bf78a05

xnv approved these changes Aug 4, 2025

View reviewed changes

kamakiri01 reviewed Aug 5, 2025

View reviewed changes

kamakiri01 approved these changes Aug 5, 2025

View reviewed changes

ShinobuTakahashi added 11 commits August 5, 2025 14:00

Fix spec

0597faf

Fix export

58e42ad

fix extra

c1f0ef9

Fix init

7be56f1

Fix scan

d671ac5

Fix lib-manage

93c7eb4

Fix BasicParametersSpec

b975719

Remove unnecessary fixes

8f0275f

Fix NodeModulesSpec

af1a076

Merge branch 'remove-browserify-use-rollup' into use-memfs-for-commons

11451dd

Merge branch 'use-memfs-for-commons' into use-memfs-other

852f952

ShinobuTakahashi mentioned this pull request Aug 27, 2025

Use memfs other #1586

Merged

ShinobuTakahashi added 4 commits August 27, 2025 10:58

Remove mock-fs

0eeb5dc

Fix Testutil

b68692a

Fix NodeModulesSpec

c4e3da8

Fix NodeModuleSpec

5ad18d6

ShinobuTakahashi and others added 5 commits September 19, 2025 11:14

Fix BasicParameterSpec

2c0ced3

Fix installSpec

d20fc16

remove unnecessary fixes

482045b

Fix comment

59d09d2

Merge pull request #1586 from akashic-games/use-memfs-other

0b0002c

Use memfs other

ShinobuTakahashi merged commit d795562 into remove-browserify-use-rollup Feb 26, 2026

ShinobuTakahashi deleted the use-memfs-for-commons branch February 26, 2026 01:34

ShinobuTakahashi mentioned this pull request Mar 3, 2026

Remove browserify, Use rollup #1561

Merged

Conversation

ShinobuTakahashi commented Jul 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

概要

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xnv Jul 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ShinobuTakahashi Jul 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xnv Jul 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xnv Aug 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

ShinobuTakahashi commented Jul 30, 2025 •

edited

Loading

xnv Jul 30, 2025 •

edited

Loading

ShinobuTakahashi Jul 30, 2025 •

edited

Loading

xnv Jul 30, 2025 •

edited

Loading

xnv Aug 1, 2025 •

edited

Loading