Use memfs other by ShinobuTakahashi · Pull Request #1586 · akashic-games/akashic-cli

ShinobuTakahashi · 2025-08-20T02:00:14Z

概要

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

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

備考
#1576 へマージします。この PR で CI が通る事を確認。
ローカルでテスト後に実ファイルが残らない事を確認。

ShinobuTakahashi · 2025-08-27T01:27:37Z

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

-export function prepareFsContent(def: FsContentDefinition, baseDir: string): PrepareFsContentResult  {
-	const dir = fs.mkdtempSync(baseDir);
+export function prepareFsContent(def: FsContentDefinition, baseDir: string, useDirPath?: string): PrepareFsContentResult  {
+	const dir = useDirPath ? useDirPath : fs.mkdtempSync(baseDir);


テストで同ディレクトリに対しファイルの追加があるため、引数 useDirPath があった場合にはそのディレクトリパスを使用するようにしました。

うーん、第二第三引数はほとんど意味が重複しているのは抵抗があります。mkdtemp() を呼び出し元の責任にしませんか。

通常:

prepareFsContent(def, fs.mkdtempSync(baseDir));

今回必要なパス:

prepareFsContent(def, path.join(fixtureContents.path, "somedir", "node_modules"));

ありがとうございます。mkdtemp() を呼び出し元の責務にするよう修正しました。

ShinobuTakahashi · 2025-08-27T01:35:44Z

packages/akashic-cli-lib-manage/src/install/__tests__/installSpec.ts

-			.then(() => promiseInstall({ moduleNames: ["dummy2@1.0.1"], cwd: "./somedir", plugin: 12, logger: logger, debugNpm: dummyNpm }))
-			.then(() => cmn.FileSystem.readJSON<cmn.GameConfiguration>("./somedir/game.json"))
+			.then(() => promiseInstall({ moduleNames: ["dummy2"], cwd: somedir, plugin: 12, logger: logger, debugNpm: dummyNpm }))
+			.then(() => cmn.FileSystem.readJSON<cmn.GameConfiguration>(path.join(somedir, "game.json")))


モジュール名の "@x.x.x" は rollup で解決できないため削除

バージョンを指定しての akashic install ができなくなるということでしょうか？

moduleName@x.x.x で install できるよう修正しました。

xnv · 2025-08-29T08:32:45Z

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

 			const moduleMainScripts = NodeModules.listModuleMainScripts(packageJsonFiles);
+
+			// CI の windows 用にファイルパスを unix 形式に変換して比較
+			for(const [key, value] of Object.entries(moduleMainScripts)) moduleMainScripts[key] = toUnixPath(value);


listModuleMainScripts() の戻り値 (game.json にそのまま書く) は常に unix path ではないかと思いますが、この処理必要でしょうか？

仰る通りです。この処理削除しました。

xnv · 2025-08-29T08:32:58Z

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

 			const moduleMainPaths = NodeModules.listModuleMainPaths(packageJsonFiles);

+			// CI の windows 用にファイルパスを unix 形式に変換して比較
+			for(const [key, value] of Object.entries(moduleMainPaths)) moduleMainPaths[key] = toUnixPath(value);


ここも。

こちらも削除しました

xnv · 2025-08-29T08:43:04Z

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

-export function prepareFsContent(def: FsContentDefinition, baseDir: string): PrepareFsContentResult  {
-	const dir = fs.mkdtempSync(baseDir);
+export function prepareFsContent(def: FsContentDefinition, baseDir: string, useDirPath?: string): PrepareFsContentResult  {
+	const dir = useDirPath ? useDirPath : fs.mkdtempSync(baseDir);


うーん、第二第三引数はほとんど意味が重複しているのは抵抗があります。mkdtemp() を呼び出し元の責任にしませんか。

通常:

prepareFsContent(def, fs.mkdtempSync(baseDir));

今回必要なパス:

prepareFsContent(def, path.join(fixtureContents.path, "somedir", "node_modules"));

xnv · 2025-08-29T08:46:39Z

packages/akashic-cli-init/src/__tests__/TemplateConfigSpec.ts

+		const mockFsContent = 	{
+				"source": {


インデントと空白見直したいです。

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

xnv · 2025-08-29T08:58:48Z

packages/akashic-cli-lib-manage/src/uninstall/__tests__/uninstallSpec.ts

+					const rmPath = path.join(fixtureContents.path, "testdir", "foo", "node_modules", name);
+					fs.rmSync(rmPath, {recursive: true});


DummyNpm は実ファイルを触らずに npm の動作を模擬するものだったように見えるのですが、ここで実ファイルを削除する必要があるんでしょうか。(だとするとこれまでなぜ動いていたのか不思議なような？)

仰る通り、ここで実ファイルを削除せずとも問題ありませんでした。修正しました。

xnv · 2025-08-29T09:01:34Z

packages/akashic-cli-lib-manage/src/install/__tests__/installSpec.ts

-			.then(() => promiseInstall({ moduleNames: ["dummy2@1.0.1"], cwd: "./somedir", plugin: 12, logger: logger, debugNpm: dummyNpm }))
-			.then(() => cmn.FileSystem.readJSON<cmn.GameConfiguration>("./somedir/game.json"))
+			.then(() => promiseInstall({ moduleNames: ["dummy2"], cwd: somedir, plugin: 12, logger: logger, debugNpm: dummyNpm }))
+			.then(() => cmn.FileSystem.readJSON<cmn.GameConfiguration>(path.join(somedir, "game.json")))


バージョンを指定しての akashic install ができなくなるということでしょうか？

xnv · 2025-08-29T09:02:59Z

packages/akashic-cli-lib-manage/src/install/__tests__/installSpec.ts

+			.then(async () => {
+				const ret = await cmn.FileSystem.readJSON<cmn.GameConfiguration>(path.join(somedir, "game.json"))
+				return ret;
+			})


Promise 返すだけなので async も await も変数もいらなくてこれでいいような？

.then(() => cmn.FileSystem.readJSON<cmn.GameConfiguration>(path.join(somedir, "game.json")))

デバッグ時に確認のため修正したままでした。await 等削除し戻しました。

kamakiri01 · 2025-09-18T08:46:40Z

packages/akashic-cli-init/src/__tests__/BasicParametersSpec.ts

+import { readJSON, writeJSON } from "@akashic/akashic-cli-commons/lib/FileSystem.js";
+
+vi.mock("node:fs", async () => {
+  const memfs: { fs: typeof fs } = await vi.importActual("memfs")


行末の ; が抜けていそうです。

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

xnv · 2025-10-03T10:36:34Z

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

 import { Util } from "..";
 import * as testUtil from "./helpers/TestUtil.js";

+const toUnixPath = (p: string) => p.replace(/^\//, "").replace(/\\/g, "/");


0 文字目の "/" を削るのは toUnixPath() という名前にそぐわないので、 toGameJsonPath() とかにしておくのはどうでしょう。

ありがとうございます。toGameJsonPath() へ修正しました。

xnv · 2025-10-03T10:49:06Z

packages/akashic-cli-lib-manage/src/install/__tests__/installSpec.ts

+				expect(content.moduleMainScripts).toEqual({ 
+					"dummy": "node_modules/dummy/index2.js",
+					"dummyChild": "node_modules/dummy/node_modules/dummyChild/main.js"
+				});


この辺り期待動作が変化しているのはなぜでしょう？

コメントアウトした箇所の影響で、dummy モジュールは初回インストールした内容のため修正しました。

xnv · 2025-10-03T10:50:34Z

packages/akashic-cli-lib-manage/src/install/__tests__/installSpec.ts

-				expect(globalScripts.indexOf("node_modules/dummy/foo.js")).toBe(-1);
-				expect(globalScripts.indexOf("node_modules/dummy/node_modules/dummyChild/main.js")).toBe(-1);
-				expect(globalScripts.indexOf("node_modules/dummy/index2.js")).not.toBe(-1);
-				expect(globalScripts.indexOf("node_modules/dummy/sub2.js")).not.toBe(-1);


このあたりもばっさり削られていますが、そうすると上の // dummyモジュールの定義を書き換えて反映されるか確認する の部分の意味がなくなっていませんか。もしこのあたりをテストする意味がないなら書き換え部分も削除されるべきに思えます。

別途ご相談させてきただき、rollup で変更したpackage.json が認識できないため、このブロックをコメントアウトとしました。

xnv

コメントつけていますが approved.

xnv · 2026-02-04T06:32:36Z

packages/akashic-cli-lib-manage/src/install/__tests__/installSpec.ts

 			})
+/*			
+			// Rollup は一度読み込んだ package.json が変更された場合、packag.json の main フィールドが変わっても検知できない。 
+			// そのため、下記の dummy モジュールの中身が変更されて反映できているかのブロックをコメントアウトとする。


厳密には --watch 中に node_modules 以下の package.json 書き換えを検知できなくなっているので、デグレである。ただ回避手段が見当たらないこと、現実的にほぼないユースケースであるため妥協する という点も書いておきたいです。

ありがとうございます。コメント追加しました。

ShinobuTakahashi added 8 commits August 6, 2025 11:04

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

ShinobuTakahashi commented Aug 27, 2025

View reviewed changes

ShinobuTakahashi added 2 commits August 27, 2025 10:53

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

852f952

Remove mock-fs

0eeb5dc

ShinobuTakahashi requested review from kamakiri01 and xnv August 27, 2025 03:21

xnv reviewed Aug 29, 2025

View reviewed changes

ShinobuTakahashi added 3 commits September 9, 2025 10:53

Fix Testutil

b68692a

Fix NodeModulesSpec

c4e3da8

Fix NodeModuleSpec

5ad18d6

kamakiri01 reviewed Sep 18, 2025

View reviewed changes

kamakiri01 approved these changes Sep 18, 2025

View reviewed changes

Fix BasicParameterSpec

2c0ced3

xnv reviewed Oct 3, 2025

View reviewed changes

ShinobuTakahashi added 2 commits February 3, 2026 13:04

Fix installSpec

d20fc16

remove unnecessary fixes

482045b

xnv approved these changes Feb 4, 2026

View reviewed changes

Fix comment

59d09d2

ShinobuTakahashi merged commit 0b0002c into use-memfs-for-commons Feb 4, 2026

ShinobuTakahashi deleted the use-memfs-other branch February 4, 2026 07:55

This was referenced Feb 26, 2026

Use memfs for commons #1576

Merged

Remove browserify, Use rollup #1561

Merged

		const rmPath = path.join(fixtureContents.path, "testdir", "foo", "node_modules", name);
		fs.rmSync(rmPath, {recursive: true});

Conversation

ShinobuTakahashi commented Aug 20, 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!

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!

ShinobuTakahashi Sep 9, 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!

xnv left a comment

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

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ShinobuTakahashi commented Aug 20, 2025 •

edited

Loading

ShinobuTakahashi Sep 9, 2025 •

edited

Loading