fix(course-format): directories are not lesson files; a failing course never rejects loadAll
Co-Authored-By: Claude Opus 5.5 <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
b8c65626f9
commit
a82ab18bee
@@ -1,5 +1,5 @@
|
||||
import { readdir, readFile, realpath } from "node:fs/promises";
|
||||
import { realpathSync } from "node:fs";
|
||||
import { realpathSync, statSync } from "node:fs";
|
||||
import { basename, dirname, isAbsolute, join, relative, resolve, sep } from "node:path";
|
||||
import { extractHints, extractSection } from "./markdown";
|
||||
import { parseManifest, type Lesson, type Manifest } from "./schema";
|
||||
@@ -67,8 +67,27 @@ export async function discoverManifests(courseDirs: string[]): Promise<string[]>
|
||||
return found;
|
||||
}
|
||||
|
||||
/** Never throws: an unexpected failure becomes an error diagnostic on that course only. */
|
||||
export async function loadCourse(manifestPath: string): Promise<LoadedCourse> {
|
||||
manifestPath = resolve(manifestPath);
|
||||
const dir = dirname(manifestPath);
|
||||
try {
|
||||
return await loadCourseUnsafe(manifestPath);
|
||||
} catch (e) {
|
||||
const id = basename(dir);
|
||||
return {
|
||||
id,
|
||||
manifestPath,
|
||||
root: dir,
|
||||
manifest: null,
|
||||
status: "error",
|
||||
diagnostics: [{ level: "error", courseId: id, path: manifestPath, message: `unexpected error loading course: ${(e as Error).message}` }],
|
||||
lessonIndex: new Map(),
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
async function loadCourseUnsafe(manifestPath: string): Promise<LoadedCourse> {
|
||||
const dir = dirname(manifestPath);
|
||||
const diagnostics: Diagnostic[] = [];
|
||||
const course: LoadedCourse = {
|
||||
@@ -118,10 +137,16 @@ export async function loadCourse(manifestPath: string): Promise<LoadedCourse> {
|
||||
if (!mdCache.has(abs)) mdCache.set(abs, await Bun.file(abs).text());
|
||||
return mdCache.get(abs)!;
|
||||
};
|
||||
/** Reports an error and returns false when `rel` does not exist inside the root. */
|
||||
const requireFile = (rel: string, what: string, lessonId?: string): boolean => {
|
||||
if (resolveInside(root, rel)) return true;
|
||||
err(`${what} not found: ${rel}`, { lessonId, path: rel });
|
||||
/** Reports an error and returns false when `rel` is not a regular file (or directory, if asked) inside the root. */
|
||||
const requireFile = (rel: string, what: string, lessonId?: string, dir = false): boolean => {
|
||||
const abs = resolveInside(root, rel);
|
||||
if (!abs) {
|
||||
err(`${what} not found: ${rel}`, { lessonId, path: rel });
|
||||
return false;
|
||||
}
|
||||
const st = statSync(abs);
|
||||
if (dir ? st.isDirectory() : st.isFile()) return true;
|
||||
err(`${what} is not a ${dir ? "directory" : "regular file"}: ${rel}`, { lessonId, path: rel });
|
||||
return false;
|
||||
};
|
||||
|
||||
@@ -131,7 +156,7 @@ export async function loadCourse(manifestPath: string): Promise<LoadedCourse> {
|
||||
for (const a of manifest.attachments ?? []) requireFile(a.file, "attachment");
|
||||
|
||||
for (const mod of manifest.modules) {
|
||||
if (mod.code) requireFile(mod.code.dir, `module "${mod.id}" code dir`);
|
||||
if (mod.code) requireFile(mod.code.dir, `module "${mod.id}" code dir`, undefined, true);
|
||||
for (const l of mod.lessons) {
|
||||
const id = l.id;
|
||||
let fileOk = true;
|
||||
|
||||
@@ -80,6 +80,34 @@ describe("overlays and errors", () => {
|
||||
expect(errs[2]!.message).toContain("Exercise 1");
|
||||
});
|
||||
|
||||
test("lesson file that is a directory gives a diagnostic", async () => {
|
||||
const d = await tmp();
|
||||
await Bun.write(join(d, "adir/x.md"), "# X\n");
|
||||
await Bun.write(join(d, "stcourse.yaml"), manifest("dirs", "", " - { id: as-file, title: L, type: text, file: adir }"));
|
||||
const c = await loadCourse(join(d, "stcourse.yaml"));
|
||||
expect(c.status).toBe("error");
|
||||
const e = c.diagnostics.filter((x) => x.level === "error");
|
||||
expect(e).toHaveLength(1);
|
||||
expect(e[0]!.lessonId).toBe("as-file");
|
||||
expect(e[0]!.path).toBe("adir");
|
||||
});
|
||||
|
||||
test("one throwing course does not break loadAll", async () => {
|
||||
const d = await tmp();
|
||||
await Bun.write(join(d, "bad/adir/x.md"), "# X\n");
|
||||
await Bun.write(join(d, "bad/a.md"), "# A\n## Sec\n");
|
||||
await Bun.write(
|
||||
join(d, "bad/stcourse.yaml"),
|
||||
manifest("bad", "", " - { id: h, title: L, type: exercise, file: a.md, hints: { file: adir, match: 'E' } }"),
|
||||
);
|
||||
await Bun.write(join(d, "good/a.md"), "# A\n");
|
||||
await Bun.write(join(d, "good/stcourse.yaml"), manifest("good", "", " - { id: l, title: L, type: text, file: a.md }"));
|
||||
const all = await loadAll([d]);
|
||||
expect(all.map((c) => c.id).sort()).toEqual(["bad", "good"]);
|
||||
expect(all.find((c) => c.id === "bad")!.status).toBe("error");
|
||||
expect(all.find((c) => c.id === "good")!.status).toBe("ok");
|
||||
});
|
||||
|
||||
test("invalid manifest gives an error course and does not throw", async () => {
|
||||
const d = await tmp();
|
||||
await Bun.write(join(d, "stcourse.yaml"), "schema: 2\nid: x\n");
|
||||
|
||||
Reference in New Issue
Block a user