fix: close housekeeping foundation review findings
This commit is contained in:
1 parent
ebc263da35
commit
08f8d54888
21 files changed
+593
-46
No files matched your search
@@ -50,6 +50,48 @@ describe("validateMigrationEntries", () => {
|
||||
expect(issues).toContain("REBUILD requires targetPath: /admin");
|
||||
});
|
||||
|
||||
it("rejects a discovery surface mismatch for the same legacy path", () => {
|
||||
expect(
|
||||
validateMigrationEntries(
|
||||
[page("/admin")],
|
||||
[{ ...entry("/admin"), surface: "mod" }],
|
||||
),
|
||||
).toEqual([
|
||||
"surface mismatch for legacyPath /admin: expected admin, received mod",
|
||||
]);
|
||||
});
|
||||
|
||||
it("rejects a discovery source file mismatch for the same legacy path", () => {
|
||||
expect(
|
||||
validateMigrationEntries(
|
||||
[page("/admin")],
|
||||
[
|
||||
{
|
||||
...entry("/admin"),
|
||||
sourceFile: "src/app/admin/renamed/page.tsx",
|
||||
},
|
||||
],
|
||||
),
|
||||
).toEqual([
|
||||
"sourceFile mismatch for legacyPath /admin: expected src/app/admin/page.tsx, received src/app/admin/renamed/page.tsx",
|
||||
]);
|
||||
});
|
||||
|
||||
it("rejects removal with a non-null target", () => {
|
||||
expect(
|
||||
validateMigrationEntries(
|
||||
[page("/admin")],
|
||||
[
|
||||
{
|
||||
...entry("/admin"),
|
||||
decision: "REMOVE",
|
||||
targetPath: "/admin/system/legacy",
|
||||
},
|
||||
],
|
||||
),
|
||||
).toEqual(["REMOVE requires null targetPath: /admin"]);
|
||||
});
|
||||
|
||||
it("enforces migration safety evidence", () => {
|
||||
const issues = validateMigrationEntries(
|
||||
[page("/admin")],
|
||||
|
||||
@@ -16,7 +16,13 @@ export function validateMigrationEntries(
|
||||
entries: readonly MigrationEntry[],
|
||||
): string[] {
|
||||
const issues = new Set<string>();
|
||||
const discoveredPaths = new Set(discovered.map((page) => page.legacyPath));
|
||||
const discoveredByPath = new Map<string, LegacyPage[]>();
|
||||
for (const page of discovered) {
|
||||
const matchingPages = discoveredByPath.get(page.legacyPath) ?? [];
|
||||
matchingPages.push(page);
|
||||
discoveredByPath.set(page.legacyPath, matchingPages);
|
||||
}
|
||||
|
||||
const entriesByPath = new Map<string, MigrationEntry[]>();
|
||||
|
||||
for (const entry of entries) {
|
||||
@@ -24,8 +30,30 @@ export function validateMigrationEntries(
|
||||
matchingEntries.push(entry);
|
||||
entriesByPath.set(entry.legacyPath, matchingEntries);
|
||||
|
||||
if (!discoveredPaths.has(entry.legacyPath)) {
|
||||
const matchingPages = discoveredByPath.get(entry.legacyPath);
|
||||
if (!matchingPages) {
|
||||
issues.add(`unknown legacyPath: ${entry.legacyPath}`);
|
||||
continue;
|
||||
}
|
||||
|
||||
const surfaceMatch = matchingPages.find(
|
||||
(page) => page.surface === entry.surface,
|
||||
);
|
||||
const identityMatch = matchingPages.some(
|
||||
(page) =>
|
||||
page.surface === entry.surface && page.sourceFile === entry.sourceFile,
|
||||
);
|
||||
if (!surfaceMatch) {
|
||||
const expectedSurface = matchingPages
|
||||
.map((page) => page.surface)
|
||||
.join(" or ");
|
||||
issues.add(
|
||||
`surface mismatch for legacyPath ${entry.legacyPath}: expected ${expectedSurface}, received ${entry.surface}`,
|
||||
);
|
||||
} else if (!identityMatch) {
|
||||
issues.add(
|
||||
`sourceFile mismatch for legacyPath ${entry.legacyPath}: expected ${surfaceMatch.sourceFile}, received ${entry.sourceFile}`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -45,6 +73,10 @@ export function validateMigrationEntries(
|
||||
for (const entry of entries) {
|
||||
const { legacyPath } = entry;
|
||||
|
||||
if (entry.decision === "REMOVE" && entry.targetPath !== null) {
|
||||
issues.add(`REMOVE requires null targetPath: ${legacyPath}`);
|
||||
}
|
||||
|
||||
if (entry.targetPath === null && entry.decision !== "REMOVE") {
|
||||
issues.add(`${entry.decision} requires targetPath: ${legacyPath}`);
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user