Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
28 changes: 28 additions & 0 deletions scripts/tests/test_pr_scoped_webviews_checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,34 @@ def test_coverage_pr_adding_an_uncovered_component_fails(self):
{'webviews/src/pages/zz/Orphan.tsx': ORPHAN})
self.assertRun(checkout.run(COVERAGE), True, 'pages/zz/Orphan.tsx#Orphan')

def test_coverage_base_gap_warns_on_a_pr_changing_the_gallery_format(self):
# format.ts only validates entries, so editing it owns entry problems, not coverage gaps (#19699).
fmt = (WEBVIEWS / 'src/gallery/format.ts').read_text() + '// edited\n'
checkout = self.checkout({'webviews/src/pages/zz/Orphan.tsx': ORPHAN},
{'webviews/src/gallery/format.ts': fmt})
self.assertRun(checkout.run(COVERAGE), False, 'pages/zz/Orphan.tsx#Orphan')

def test_coverage_pr_changing_the_entry_constructors_owns_every_finding(self):
# componentEntry and the other constructors build the covers the coverage tests read.
entry = (WEBVIEWS / 'src/gallery/entry.ts').read_text() + '// edited\n'
checkout = self.checkout({'webviews/src/pages/zz/Orphan.tsx': ORPHAN},
{'webviews/src/gallery/entry.ts': entry})
self.assertRun(checkout.run(COVERAGE), True, 'pages/zz/Orphan.tsx#Orphan')

def test_coverage_pr_changing_the_gallery_format_owns_entry_problems(self):
settings = 'webviews/src/pages/settings/components'
duplicate = (WEBVIEWS / 'src/pages/settings/components/ThemeTokens.gallery.tsx').read_text()
fmt = (WEBVIEWS / 'src/gallery/format.ts').read_text() + '// edited\n'
checkout = self.checkout({f'{settings}/AThemeTokens.gallery.tsx': duplicate},
{'webviews/src/gallery/format.ts': fmt})
self.assertRun(checkout.run(COVERAGE), True, 'pages.settings.theme-tokens: duplicate id')

def test_coverage_pr_changing_the_check_owns_every_finding(self):
check = (WEBVIEWS / COVERAGE).read_text() + '// edited\n'
checkout = self.checkout({'webviews/src/pages/zz/Orphan.tsx': ORPHAN},
{f'webviews/{COVERAGE}': check})
self.assertRun(checkout.run(COVERAGE), True, 'pages/zz/Orphan.tsx#Orphan')

def test_coverage_pr_dropping_an_allowlist_line_owns_it(self):
allowlist = WEBVIEWS / 'test/gallery-coverage.allowlist.json'
text = allowlist.read_text()
Expand Down
72 changes: 72 additions & 0 deletions webviews/src/gallery/entry.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
// The identity helpers every gallery file builds its entry with. What they return is the entry
// the gallery coverage check reads (covers, id), so editing them owns every coverage finding
// (test/gallery-coverage.test.ts); format.ts re-exports them.
import type {
AgentPaneEntry,
AppsPageEntry,
ChangelogPageEntry,
CloudPageEntry,
CodeRouterPageEntry,
ComponentEntry,
DiffPageEntry,
EditorPageEntry,
HistoryPageEntry,
IconPickerPageEntry,
KeybindingsPageEntry,
MarkdownPageEntry,
NativeEntry,
PasswordsPageEntry,
SettingsPageEntry,
} from "./format";

export const settingsPageEntry = (entry: Omit<SettingsPageEntry, "host">): SettingsPageEntry => ({
...entry,
host: "settings-page",
});
export const passwordsPageEntry = (entry: Omit<PasswordsPageEntry, "host">): PasswordsPageEntry => ({
...entry,
host: "passwords-page",
});
/** Identity helpers that check an entry against its host's variant type. */
export const agentPaneEntry = (entry: Omit<AgentPaneEntry, "host">): AgentPaneEntry => ({
...entry,
host: "agent-pane",
});
export const markdownPageEntry = (entry: Omit<MarkdownPageEntry, "host">): MarkdownPageEntry => ({
...entry,
host: "markdown-page",
});
export const diffPageEntry = (entry: Omit<DiffPageEntry, "host">): DiffPageEntry => ({ ...entry, host: "diff-page" });
export const appsPageEntry = (entry: Omit<AppsPageEntry, "host">): AppsPageEntry => ({ ...entry, host: "apps-page" });
export const cloudPageEntry = (entry: Omit<CloudPageEntry, "host">): CloudPageEntry => ({
...entry,
host: "cloud-page",
});
export const codeRouterPageEntry = (entry: Omit<CodeRouterPageEntry, "host">): CodeRouterPageEntry => ({
...entry,
host: "coderouter-page",
});
export const changelogPageEntry = (entry: Omit<ChangelogPageEntry, "host">): ChangelogPageEntry => ({
...entry,
host: "changelog-page",
});
export const iconPickerPageEntry = (entry: Omit<IconPickerPageEntry, "host">): IconPickerPageEntry => ({
...entry,
host: "icon-picker-page",
});
export const editorPageEntry = (entry: Omit<EditorPageEntry, "host">): EditorPageEntry => ({
...entry,
host: "editor-page",
});
export const historyPageEntry = (entry: Omit<HistoryPageEntry, "host">): HistoryPageEntry => ({
...entry,
host: "history-page",
});
export const keybindingsPageEntry = (entry: Omit<KeybindingsPageEntry, "host">): KeybindingsPageEntry => ({
...entry,
host: "keybindings-page",
});
export function componentEntry<P>(entry: Omit<ComponentEntry<P>, "host">): ComponentEntry<P> {
return { ...entry, host: "component" };
}
export const nativeEntry = (entry: Omit<NativeEntry, "host">): NativeEntry => ({ ...entry, host: "native" });
68 changes: 17 additions & 51 deletions webviews/src/gallery/format.ts
Original file line number Diff line number Diff line change
Expand Up @@ -362,59 +362,25 @@ export type GalleryEntry =
| PasswordsPageEntry
| ComponentEntry<any>
| NativeEntry;
export const settingsPageEntry = (entry: Omit<SettingsPageEntry, "host">): SettingsPageEntry => ({
...entry,
host: "settings-page",
});
export const passwordsPageEntry = (entry: Omit<PasswordsPageEntry, "host">): PasswordsPageEntry => ({
...entry,
host: "passwords-page",
});
export type HostKind = GalleryEntry["host"];

/** Identity helpers that check an entry against its host's variant type. */
export const agentPaneEntry = (entry: Omit<AgentPaneEntry, "host">): AgentPaneEntry => ({
...entry,
host: "agent-pane",
});
export const markdownPageEntry = (entry: Omit<MarkdownPageEntry, "host">): MarkdownPageEntry => ({
...entry,
host: "markdown-page",
});
export const diffPageEntry = (entry: Omit<DiffPageEntry, "host">): DiffPageEntry => ({ ...entry, host: "diff-page" });
export const appsPageEntry = (entry: Omit<AppsPageEntry, "host">): AppsPageEntry => ({ ...entry, host: "apps-page" });
export const cloudPageEntry = (entry: Omit<CloudPageEntry, "host">): CloudPageEntry => ({
...entry,
host: "cloud-page",
});
export const codeRouterPageEntry = (entry: Omit<CodeRouterPageEntry, "host">): CodeRouterPageEntry => ({
...entry,
host: "coderouter-page",
});
export const changelogPageEntry = (entry: Omit<ChangelogPageEntry, "host">): ChangelogPageEntry => ({
...entry,
host: "changelog-page",
});
export const iconPickerPageEntry = (entry: Omit<IconPickerPageEntry, "host">): IconPickerPageEntry => ({
...entry,
host: "icon-picker-page",
});
export const editorPageEntry = (entry: Omit<EditorPageEntry, "host">): EditorPageEntry => ({
...entry,
host: "editor-page",
});
export const historyPageEntry = (entry: Omit<HistoryPageEntry, "host">): HistoryPageEntry => ({
...entry,
host: "history-page",
});
export const keybindingsPageEntry = (entry: Omit<KeybindingsPageEntry, "host">): KeybindingsPageEntry => ({
...entry,
host: "keybindings-page",
});
export function componentEntry<P>(entry: Omit<ComponentEntry<P>, "host">): ComponentEntry<P> {
return { ...entry, host: "component" };
}
export const nativeEntry = (entry: Omit<NativeEntry, "host">): NativeEntry => ({ ...entry, host: "native" });
export {
settingsPageEntry,
passwordsPageEntry,
agentPaneEntry,
markdownPageEntry,
diffPageEntry,
appsPageEntry,
cloudPageEntry,
codeRouterPageEntry,
changelogPageEntry,
iconPickerPageEntry,
editorPageEntry,
historyPageEntry,
keybindingsPageEntry,
componentEntry,
nativeEntry,
} from "./entry";

export const DEFAULT_STAGE_HEIGHT = 520;

Expand Down
22 changes: 15 additions & 7 deletions webviews/test/gallery-coverage.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,18 +66,26 @@ function pages(): Map<string, string[]> {
}

const pageFiles = pages();
const scope = prScope({
const RULES = [
"webviews/test/gallery-coverage.test.ts",
"webviews/test/support/prScope.ts",
"webviews/scripts/gallery/entries.ts",
// The entry constructors build the covers and ids every coverage test reads.
"webviews/src/gallery/entry.ts",
];
const LISTS = ["webviews/test/gallery-coverage.allowlist.json", /^webviews\/src\/.*\.gallery\.tsx?$/];
const scope = prScope({ rules: RULES, lists: LISTS });
// validateEntries and its helpers decide only "the entries are valid": a pull request that edits
// them owns every entry problem, not the base branch's coverage gaps.
const validation = prScope({
rules: [
"webviews/test/gallery-coverage.test.ts",
"webviews/test/support/prScope.ts",
"webviews/scripts/gallery/entries.ts",
...RULES,
"webviews/src/gallery/format.ts",
// validateEntries' helpers.
"webviews/src/gallery/play.ts",
"webviews/src/experiments/experiment.ts",
"webviews/src/experiments/tunable.ts",
],
lists: ["webviews/test/gallery-coverage.allowlist.json", /^webviews\/src\/.*\.gallery\.tsx?$/],
lists: LISTS,
});
/** A component (`file#Name`) or page id: its source files, and the names an allowlist or covers line gives it. */
function place(name: string) {
Expand Down Expand Up @@ -153,7 +161,7 @@ describe("gallery coverage", async () => {
const allowlist = JSON.parse(fs.readFileSync(ALLOWLIST, "utf8")) as Allowlist;

test("the entries are valid", () => {
expect(scope.owned(validateEntries(entries), entryPlace, (problem) => problem)).toEqual([]);
expect(validation.owned(validateEntries(entries), entryPlace, (problem) => problem)).toEqual([]);
});

test("every cover names a component or page that exists", () => {
Expand Down
Loading