Skip to content

test: reload config when imported file changes #34

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.

Sign up for GitHub

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

Merged
chenjiahan merged 1 commit into from
Jul 12, 2026
Merged

test: reload config when imported file changes #34

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
25 changes: 25 additions & 0 deletions packages/rstack/test/config/reload-app-config/index.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,3 +48,28 @@ define.app({

await expectFile(dist2);
}, 30_000);

test('should reload config when an imported file changes', async ({ execCliAsync, logHelper }) => {
const configFile = path.join(import.meta.dirname, 'test-temp-import.config.ts');
const importedFile = path.join(import.meta.dirname, 'test-temp-imported.ts');

await writeFile(importedFile, '');
await writeFile(
configFile,
`import { define } from 'rstack';
import './test-temp-imported.ts';

@chatgpt-codex-connector chatgpt-codex-connector Bot Jul 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Assert the changed import affects the config

Because the imported file is only loaded for side effects and the assertion only waits for the restart log, this test can pass even if the dev server notices the dependency but reloads the config with stale imported values. In the scenario this is meant to cover—an imported config helper changes—the regression would still slip through; make the imported module export an observable config value, change it, and assert the post-restart build/server reflects the new value.

Useful? React with 👍 / 👎.


define.app({
server: { port: ${await getRandomPort()} },
});
`,
);

execCliAsync('dev --config test-temp-import.config.ts');
await logHelper.expectBuildEnd();
logHelper.clearLogs();

await writeFile(importedFile, '// changed\n');

await logHelper.expectLog('restarting server as test-temp-imported.ts changed');
}, 30_000);
Comment on lines +52 to +75

@coderabbitai coderabbitai Bot Jul 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Add cleanup of temp files before and after the test.

The new test creates test-temp-import.config.ts and test-temp-imported.ts but never removes them. The first test in this file cleans up its temp files at the start (lines 12–14); this test should follow the same pattern and also clean up after the test to avoid leaving artifacts on success or failure. Stale files from a failed previous run could interfere with file watchers or import resolution.

🧹 Proposed fix: add cleanup at start and in finally block 
 test('should reload config when an imported file changes', async ({ execCliAsync, logHelper }) => {
   const configFile = path.join(import.meta.dirname, 'test-temp-import.config.ts');
   const importedFile = path.join(import.meta.dirname, 'test-temp-imported.ts');
 
+  await rm(configFile, { force: true });
+  await rm(importedFile, { force: true });
+
+  try {
     await writeFile(importedFile, '');
     await writeFile(
       configFile,
       `import { define } from 'rstack';
 import './test-temp-imported.ts';
 
 define.app({
   server: { port: ${await getRandomPort()} },
 });
 `,
     );
 
     execCliAsync('dev --config test-temp-import.config.ts');
     await logHelper.expectBuildEnd();
     logHelper.clearLogs();
 
     await writeFile(importedFile, '// changed\n');
 
     await logHelper.expectLog('restarting server as test-temp-imported.ts changed');
+  } finally {
+    await rm(configFile, { force: true });
+    await rm(importedFile, { force: true });
   }
 }, 30_000);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
test('should reload config when an imported file changes', async ({ execCliAsync, logHelper }) => {
const configFile = path.join(import.meta.dirname, 'test-temp-import.config.ts');
const importedFile = path.join(import.meta.dirname, 'test-temp-imported.ts');
await writeFile(importedFile, '');
await writeFile(
configFile,
`import { define } from 'rstack';
import './test-temp-imported.ts';
define.app({
server: { port: ${await getRandomPort()} },
});
`,
);
execCliAsync('dev --config test-temp-import.config.ts');
await logHelper.expectBuildEnd();
logHelper.clearLogs();
await writeFile(importedFile, '// changed\n');
await logHelper.expectLog('restarting server as test-temp-imported.ts changed');
}, 30_000);
test('should reload config when an imported file changes', async ({ execCliAsync, logHelper }) => {
const configFile = path.join(import.meta.dirname, 'test-temp-import.config.ts');
const importedFile = path.join(import.meta.dirname, 'test-temp-imported.ts');
await rm(configFile, { force: true });
await rm(importedFile, { force: true });
try {
await writeFile(importedFile, '');
await writeFile(
configFile,
`import { define } from 'rstack';
import './test-temp-imported.ts';
define.app({
server: { port: ${await getRandomPort()} },
});
`,
);
execCliAsync('dev --config test-temp-import.config.ts');
await logHelper.expectBuildEnd();
logHelper.clearLogs();
await writeFile(importedFile, '// changed\n');
await logHelper.expectLog('restarting server as test-temp-imported.ts changed');
} finally {
await rm(configFile, { force: true });
await rm(importedFile, { force: true });
}
}, 30_000);
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/rstack/test/config/reload-app-config/index.test.ts` around lines 52
- 75, Update the test case around “should reload config when an imported file
changes” to remove test-temp-import.config.ts and test-temp-imported.ts before
setup, then wrap execution in a finally block that deletes both files after
completion or failure. Follow the existing cleanup pattern from the first test
and preserve the current watcher assertions.