Skip to content

ci: run the full offline test suite and auto-rebuild stale dist before tests #212

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
sherifahmed990 merged 2 commits into from
Jun 12, 2026
Merged

ci: run the full offline test suite and auto-rebuild stale dist before tests #212

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6babbf7
ci: run the full offline test suite and auto-rebuild stale dist befor…
Sednaoui Jun 11, 2026
e637bfa
Merge branch 'dev' into ci/offline-tests
sherifahmed990 Jun 12, 2026
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
7 changes: 5 additions & 2 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,5 +45,8 @@ jobs:
- name: Build
run: yarn build

- name: Signer unit tests
run: yarn test:signer
# Runs every suite outside test/integration/ (root jest config
# excludes it): transport, safe, calibur, paymaster, signer, and
# utility tests. All offline — mock transports or loopback servers.
- name: Offline unit tests
run: yarn test
2 changes: 1 addition & 1 deletion .github/workflows/integration.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ concurrency:

jobs:
integration:
name: integration/chainId
name: integration
runs-on: ubuntu-latest
strategy:
matrix:
Expand Down
2 changes: 2 additions & 0 deletions jest.config.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,6 @@ module.exports = {
testEnvironment: "node",
modulePathIgnorePatterns: ["/.worktrees/"],
testPathIgnorePatterns: ["/node_modules/", "/dist/", "/.worktrees/", "/test/integration/"],
// Tests import the built package from dist/; rebuild when src/ is newer.
globalSetup: "<rootDir>/test/_ensureFreshDist.cjs",
};
34 changes: 34 additions & 0 deletions test/_ensureFreshDist.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
// Jest globalSetup: tests import the built package from dist/, so a stale
// build silently tests old code. Rebuild automatically when any file under
// src/ (or the build inputs) is newer than dist/index.cjs. Costs nothing
// when dist is fresh; one tsdown run (~3s) when it is not.

const { execSync } = require("node:child_process");
const fs = require("node:fs");
const path = require("node:path");

const ROOT = path.join(__dirname, "..");

function newestMtimeMs(entry) {
const stat = fs.statSync(entry);
if (!stat.isDirectory()) {
return stat.mtimeMs;
}
let newest = 0;
for (const name of fs.readdirSync(entry)) {
newest = Math.max(newest, newestMtimeMs(path.join(entry, name)));
}
return newest;
Comment on lines +17 to +21

@chatgpt-codex-connector chatgpt-codex-connector Bot Jun 11, 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 Include directory mtimes when checking stale dist

When a source file is deleted or renamed without touching another input, this recursion ignores directory mtimes and only considers the remaining files. In that refactor scenario the containing directory's mtime is the only timestamp newer than dist/index.cjs, so globalSetup skips the rebuild and targeted Jest runs keep testing a bundle that still contains the removed code. Seed the directory case with stat.mtimeMs (or otherwise include directory mtimes) so deletions invalidate dist/ too.

Useful? React with 👍 / 👎.

}
Comment on lines +12 to +22

@coderabbitai coderabbitai Bot Jun 11, 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.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Add symlink protection to prevent infinite loops.

The recursive directory traversal does not check for symbolic links before descending. If src/ or any subdirectory contains a symlink that creates a cycle, newestMtimeMs will loop infinitely and hang the test suite.

🛡️ Proposed fix to skip symlinks 
 function newestMtimeMs(entry) {
 	const stat = fs.statSync(entry);
-	if (!stat.isDirectory()) {
+	if (stat.isSymbolicLink() || !stat.isDirectory()) {
 		return stat.mtimeMs;
 	}
 	let newest = 0;

Alternatively, use lstatSync instead of statSync to avoid following symlinks:

 function newestMtimeMs(entry) {
-	const stat = fs.statSync(entry);
+	const stat = fs.lstatSync(entry);
-	if (!stat.isDirectory()) {
+	if (stat.isSymbolicLink() || !stat.isDirectory()) {
 		return stat.mtimeMs;
 	}
 	let newest = 0;
📝 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
function newestMtimeMs(entry) {
const stat = fs.statSync(entry);
if (!stat.isDirectory()) {
return stat.mtimeMs;
}
let newest = 0;
for (const name of fs.readdirSync(entry)) {
newest = Math.max(newest, newestMtimeMs(path.join(entry, name)));
}
return newest;
}
function newestMtimeMs(entry) {
const stat = fs.lstatSync(entry);
if (stat.isSymbolicLink() || !stat.isDirectory()) {
return stat.mtimeMs;
}
let newest = 0;
for (const name of fs.readdirSync(entry)) {
newest = Math.max(newest, newestMtimeMs(path.join(entry, name)));
}
return newest;
}
🤖 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 `@test/_ensureFreshDist.cjs` around lines 12 - 22, The recursive function
newestMtimeMs currently follows symlinks and can loop on cycles; change its file
checks to use fs.lstatSync instead of fs.statSync and skip entries where
lstat.isSymbolicLink() is true before recursing, so in newestMtimeMs call
lstatSync on entry to decide directory vs file and when iterating
fs.readdirSync(entry) lstatSync each child and continue (skip) if
isSymbolicLink(); keep existing mtimeMs handling for regular files/directories.


module.exports = async () => {
const distEntry = path.join(ROOT, "dist", "index.cjs");
const inputs = ["src", "package.json", "tsconfig.json", "tsdown.config.ts"]
.map((p) => path.join(ROOT, p))
.filter((p) => fs.existsSync(p));
const newestInput = Math.max(...inputs.map(newestMtimeMs));
if (!fs.existsSync(distEntry) || fs.statSync(distEntry).mtimeMs < newestInput) {
console.log("\ndist/ is stale relative to src/ — rebuilding before tests...");
execSync("npm run build", { stdio: "inherit", cwd: ROOT });
}
};
Loading