test(cli): migrate from memfs mock to fs-fixture
#457
Conversation
ryoppippi
changed the title
memfs mock to fs-fixture
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
@agentica/benchmark
@agentica/chat
agentica
@agentica/core
create-agentica
@agentica/rpc
@agentica/vector-selector
commit: |
|
too freaky... |
|
OK so this is because the template is updated to openai5, while this repo still using openai sdk 4 |
|
fixed nestia template. |
There was a problem hiding this comment.
Pull Request Overview
Migrate tests in the CLI package from using memfs to fs-fixture for more intuitive filesystem fixtures and bump test concurrency.
- Added
maxConcurrencysetting to Vitest config. - Replaced
memfsmocks and APIs in tests withfs-fixture. - Updated
package.jsonto removememfsand addfs-fixture, and removed manual__mocks__files.
Reviewed Changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| vitest.config.mts | Increased test runner concurrency with maxConcurrency: 10. |
| src/fs.test.ts | Swapped out memfs usage for fs-fixture in unit tests. |
| src/commands/start.test.ts | Converted integration tests to use fs-fixture and removed manual tmp dir logic. |
| package.json | Removed memfs dependency and added fs-fixture. |
| mocks/fs/promises.cjs & fs.cjs | Removed legacy memfs mock files. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
| describe("setupStandAloneProject", () => { | ||
| it("should create a new directory and set project .env file", async () => { | ||
| const destinationDirectory = resolve(tmpParentDirectory, ".tmp/standalone"); | ||
| /** create a fixture */ | ||
| await using fixture = await createFixture(); |
There was a problem hiding this comment.
[nitpick] Inconsistent fixture cleanup: some tests use await using while others use a plain const fixture = await createFixture() without automatic disposal. Consider standardizing on one pattern so resources are managed uniformly.
Copilot uses AI. Check for mistakes.
| // check if the directory exists | ||
| expect(existsSync(destinationDirectory)).toBe(true); | ||
| expect(existsSync(resolve(destinationDirectory, "package.json"))).toBe(true); | ||
| expect(await fixture.exists()).toBe(true); |
There was a problem hiding this comment.
This assertion only verifies that the fixture root directory (created by createFixture) exists by default. It doesn't confirm that the command actually generated the intended project files—assert on specific paths or files instead (e.g. fixture.exists('package.json')).
Copilot uses AI. Check for mistakes.
| const content = vol.readFileSync("/my-new-project/.env", "utf-8"); | ||
|
|
||
| expect(content).toBe("OPENAI_API_KEY=sk-foo"); | ||
| expect (await fixture.readFile(".env", "utf-8")).toBe("OPENAI_API_KEY=sk-foo"); |
There was a problem hiding this comment.
[nitpick] Remove the space between expect and the opening parenthesis for consistent test styling: use expect(await fixture.readFile(...)).
| expect (await fixture.readFile(".env", "utf-8")).toBe("OPENAI_API_KEY=sk-foo"); | |
| expect(await fixture.readFile(".env", "utf-8")).toBe("OPENAI_API_KEY=sk-foo"); |
Copilot uses AI. Check for mistakes.
Migrate from memfs to
fs-fixturebecause it is more intuitive!https://github.com/privatenumber/fs-fixture/tree/master