diff --git a/scripts/validate-tutor-course-content.mjs b/scripts/validate-tutor-course-content.mjs index 2788bca8..403ed03c 100644 --- a/scripts/validate-tutor-course-content.mjs +++ b/scripts/validate-tutor-course-content.mjs @@ -2,7 +2,7 @@ import path from "node:path"; import { validateTutorCourseContentDirectory } from "../server/services/course-content/tutor-quality-directory-validator.ts"; const directory = process.argv[2]; -if (!directory) { +if (directory === undefined || directory.length === 0) { console.error("Usage: npm run validate:tutor-course-content -- "); process.exitCode = 2; } else { @@ -13,7 +13,8 @@ if (!directory) { } else { for (const issue of issues) { const scope = [issue.caseId, issue.topicId, issue.conceptId, issue.indicatorId].filter(Boolean).join("/"); - console.error(`${issue.code}${scope ? ` [${scope}]` : ""}: ${issue.message}`); + const scopeSuffix = scope ? ` [${scope}]` : ""; + console.error(`${issue.code}${scopeSuffix}: ${issue.message}`); } process.exitCode = 1; } diff --git a/server/services/course-content/tutor-quality-directory-validator.ts b/server/services/course-content/tutor-quality-directory-validator.ts index 474c8a4c..31ddae99 100644 --- a/server/services/course-content/tutor-quality-directory-validator.ts +++ b/server/services/course-content/tutor-quality-directory-validator.ts @@ -1,7 +1,6 @@ import { readFile } from "node:fs/promises"; import path from "node:path"; import { parse as parseYaml } from "yaml"; -import type { FullCommitSha, RepositorySlug } from "@shared/examples"; import { CourseContentLoader, type LoadedCourseContentSnapshot } from "./course-content-loader"; import { tutorQualityCasesSchema } from "./tutor-quality-schema"; import { BUILT_IN_TUTOR_STRATEGY, type EffectiveTutorStrategy } from "../tutor/strategy/effective-tutor-strategy"; @@ -11,8 +10,8 @@ import { type TutorContentQualityIssue, } from "./tutor-quality-validator"; -const LOCAL_REVISION = "0".repeat(40) as FullCommitSha; -const LOCAL_REPOSITORY = "local/course-content" as RepositorySlug; +const LOCAL_REVISION = "0".repeat(40); +const LOCAL_REPOSITORY = "local/course-content"; export async function validateTutorCourseContentDirectory(directory: string): Promise { const root = path.resolve(directory); diff --git a/server/services/course-content/tutor-quality-validator.ts b/server/services/course-content/tutor-quality-validator.ts index 711eaf49..fccc893e 100644 --- a/server/services/course-content/tutor-quality-validator.ts +++ b/server/services/course-content/tutor-quality-validator.ts @@ -83,20 +83,46 @@ function validateActivationReferences( issues: TutorContentQualityIssue[], ): void { for (const { qualityCase, activatedTopicIds } of contexts) { - for (const topicId of [...qualityCase.expectedTopics, ...qualityCase.forbiddenTopics]) { - if (!topicIds.has(topicId)) { - issues.push(issue("unknown-topic-reference", `Quality case references unknown Topic ${topicId}`, qualityCase.id, topicId)); - } + validateUnknownTopicReferences(qualityCase, topicIds, issues); + validateExpectedTopicActivation(qualityCase, activatedTopicIds, topicIds, issues); + validateForbiddenTopicActivation(qualityCase, activatedTopicIds, topicIds, issues); + } +} + +function validateUnknownTopicReferences( + qualityCase: ResolvedTutorQualityCase, + topicIds: ReadonlySet, + issues: TutorContentQualityIssue[], +): void { + for (const topicId of [...qualityCase.expectedTopics, ...qualityCase.forbiddenTopics]) { + if (!topicIds.has(topicId)) { + issues.push(issue("unknown-topic-reference", `Quality case references unknown Topic ${topicId}`, qualityCase.id, topicId)); } - for (const topicId of qualityCase.expectedTopics) { - if (topicIds.has(topicId) && !activatedTopicIds.has(topicId)) { - issues.push(issue("expected-topic-not-activated", `Expected Topic ${topicId} is not activated`, qualityCase.id, topicId)); - } + } +} + +function validateExpectedTopicActivation( + qualityCase: ResolvedTutorQualityCase, + activatedTopicIds: ReadonlySet, + topicIds: ReadonlySet, + issues: TutorContentQualityIssue[], +): void { + for (const topicId of qualityCase.expectedTopics) { + if (topicIds.has(topicId) && !activatedTopicIds.has(topicId)) { + issues.push(issue("expected-topic-not-activated", `Expected Topic ${topicId} is not activated`, qualityCase.id, topicId)); } - for (const topicId of qualityCase.forbiddenTopics) { - if (topicIds.has(topicId) && activatedTopicIds.has(topicId)) { - issues.push(issue("forbidden-topic-activated", `Forbidden Topic ${topicId} is activated`, qualityCase.id, topicId)); - } + } +} + +function validateForbiddenTopicActivation( + qualityCase: ResolvedTutorQualityCase, + activatedTopicIds: ReadonlySet, + topicIds: ReadonlySet, + issues: TutorContentQualityIssue[], +): void { + for (const topicId of qualityCase.forbiddenTopics) { + if (topicIds.has(topicId) && activatedTopicIds.has(topicId)) { + issues.push(issue("forbidden-topic-activated", `Forbidden Topic ${topicId} is activated`, qualityCase.id, topicId)); } } } diff --git a/tests/server/services/course-content/tutor-quality-directory-validator.test.ts b/tests/server/services/course-content/tutor-quality-directory-validator.test.ts index caaf11df..e9e0de81 100644 --- a/tests/server/services/course-content/tutor-quality-directory-validator.test.ts +++ b/tests/server/services/course-content/tutor-quality-directory-validator.test.ts @@ -47,6 +47,14 @@ describe("Tutor Course Content directory validator", () => { { cwd: process.cwd() }, )).rejects.toMatchObject({ code: 1 }); }); + + it("returns usage status when no Course Content directory is provided", async () => { + await expect(executeFile( + path.resolve("node_modules/.bin/tsx"), + ["scripts/validate-tutor-course-content.mjs"], + { cwd: process.cwd() }, + )).rejects.toMatchObject({ code: 2 }); + }); }); async function courseContentDirectory(withCases = true): Promise { diff --git a/tests/server/services/course-content/tutor-quality-validator.test.ts b/tests/server/services/course-content/tutor-quality-validator.test.ts index 7a82b644..0bbf70f9 100644 --- a/tests/server/services/course-content/tutor-quality-validator.test.ts +++ b/tests/server/services/course-content/tutor-quality-validator.test.ts @@ -106,6 +106,15 @@ describe("deterministic Tutor Course Content quality", () => { ])); }); + it("reports quality cases that reference an unknown Topic", () => { + const unknownTopicCase: ResolvedTutorQualityCase = { + ...cases()[0]!, + expectedTopics: ["missing-topic"], + }; + + expect(codes(validateTutorContentQuality([validTopic()], [unknownTopicCase]))).toContain("unknown-topic-reference"); + }); + it("reports Concepts and required Indicators with no applicable question", () => { const topic = validTopic(); topic.concepts.push({