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
9 changes: 6 additions & 3 deletions packages/kbn-docs-utils/src/build_api_docs_cli.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,13 @@ describe('build_api_docs_cli', () => {
});

it('runs build flow when stats are not provided', async () => {
const mockPlugins = [
{ id: 'p1', manifest: { owner: { name: 'team' }, serviceFolders: [] }, isPlugin: true },
];
const setupResult = {
project: {},
plugins: [
{ id: 'p1', manifest: { owner: { name: 'team' }, serviceFolders: [] }, isPlugin: true },
],
plugins: mockPlugins,
allPlugins: mockPlugins,
};
const apiMapResult = {
pluginApiMap: {},
Expand All @@ -107,6 +109,7 @@ describe('build_api_docs_cli', () => {
expect(buildApiMap).toHaveBeenCalledWith(
setupResult.project,
setupResult.plugins,
setupResult.allPlugins,
log,
mockTx,
{ stats: undefined, collectReferences: false }
Expand Down
1 change: 1 addition & 0 deletions packages/kbn-docs-utils/src/build_api_docs_cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ export function runBuildApiDocsCli() {
const apiMapResult = buildApiMap(
setupResult.project,
setupResult.plugins,
setupResult.allPlugins,
log,
transaction,
options
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ describe('runCheckPackageDocs', () => {
expect(reportMetrics).toHaveBeenCalled();
expect(process.exitCode).toBe(1);
expect(log.error).toHaveBeenCalledWith(
expect.stringContaining('Validation failed for 1 plugin')
expect.stringContaining('Validation failed for 1 package')
);
expect(mockTx.end).toHaveBeenCalled();
});
Expand Down Expand Up @@ -129,7 +129,7 @@ describe('runCheckPackageDocs', () => {
await runCheckPackageDocs(log as any, { plugin: 'plugin-a' } as any);

expect(process.exitCode).toBeUndefined();
expect(log.info).toHaveBeenCalledWith('All plugins passed validation.');
expect(log.info).toHaveBeenCalledWith('All packages passed validation.');
expect(mockTx.end).toHaveBeenCalled();
});
});
7 changes: 4 additions & 3 deletions packages/kbn-docs-utils/src/check_package_docs_cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ export const runCheckPackageDocs = async (log: ToolingLog, flags: CliFlags) => {
const apiMapResult = buildApiMap(
setupResult.project,
setupResult.plugins,
setupResult.allPlugins,
log,
transaction,
optionsWithChecks
Expand Down Expand Up @@ -160,13 +161,13 @@ export const runCheckPackageDocs = async (log: ToolingLog, flags: CliFlags) => {

if (failingPlugins.length > 0) {
log.error(
`Validation failed for ${failingPlugins.length} plugin(s): ${failingPlugins
.map((plugin) => plugin.pluginId)
`Validation failed for ${failingPlugins.length} package(s): ${failingPlugins
.map(({ pluginId }) => pluginId)
.join(', ')}.`
);
process.exitCode = 1;
} else {
log.info('All plugins passed validation.');
log.info('All packages passed validation.');
}
} catch (error) {
transaction?.setOutcome('failure');
Expand Down
9 changes: 5 additions & 4 deletions packages/kbn-docs-utils/src/cli/tasks/build_api_map.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,10 @@ describe('buildApiMap', () => {
pluginFilter: ['test-plugin'],
};

buildApiMap(project, plugins, log, transaction, options);
const allPlugins = [...plugins, { id: 'other-plugin' }];
buildApiMap(project, plugins, allPlugins, log, transaction, options);

expect(getPluginApiMap).toHaveBeenCalledWith(project, plugins, log, {
expect(getPluginApiMap).toHaveBeenCalledWith(project, plugins, allPlugins, log, {
collectReferences: true,
pluginFilter: ['test-plugin'],
});
Expand All @@ -80,7 +81,7 @@ describe('buildApiMap', () => {
collectReferences: false,
};

const result = buildApiMap(project, plugins, log, transaction, options);
const result = buildApiMap(project, plugins, plugins, log, transaction, options);

expect(result).toBeDefined();
expect(result.pluginApiMap).toBeDefined();
Expand All @@ -95,7 +96,7 @@ describe('buildApiMap', () => {
collectReferences: false,
};

buildApiMap(project, plugins, log, transaction, options);
buildApiMap(project, plugins, plugins, log, transaction, options);

expect(transaction.startSpan).toHaveBeenCalledWith('build_api_docs.getPluginApiMap', 'setup');
});
Expand Down
4 changes: 3 additions & 1 deletion packages/kbn-docs-utils/src/cli/tasks/build_api_map.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import type { BuildApiMapResult, CliOptions } from '../types';
*
* @param project - TypeScript project instance.
* @param plugins - List of plugins and packages to analyze.
* @param allPlugins - All plugins/packages for cross-reference resolution.
* @param log - Tooling log instance.
* @param transaction - APM transaction for tracking.
* @param options - CLI options including collectReferences and pluginFilter.
Expand All @@ -33,6 +34,7 @@ import type { BuildApiMapResult, CliOptions } from '../types';
export function buildApiMap(
project: Project,
plugins: PluginOrPackage[],
allPlugins: PluginOrPackage[],
log: ToolingLog,
transaction: Transaction,
options: CliOptions
Expand All @@ -46,7 +48,7 @@ export function buildApiMap(
referencedDeprecations,
adoptionTrackedAPIs,
unnamedExports,
} = getPluginApiMap(project, plugins, log, {
} = getPluginApiMap(project, plugins, allPlugins, log, {
collectReferences: options.collectReferences,
pluginFilter: options.pluginFilter,
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ describe('collectStats', () => {
plugins: [mockPlugin],
pathsByPlugin: new Map([[mockPlugin, ['src/plugins/test/public/index.ts']]]),
project: {} as any,
allPlugins: [mockPlugin],
};

apiMapResult = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ describe('reportMetrics', () => {
plugins: [mockPlugin],
pathsByPlugin: new Map(),
project: {} as any,
allPlugins: [mockPlugin],
};

apiMapResult = {
Expand Down
71 changes: 50 additions & 21 deletions packages/kbn-docs-utils/src/cli/tasks/report_metrics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,17 @@ import type { AllPluginStats, BuildApiMapResult, CliOptions, SetupProjectResult
/**
* Generates a link to the GitHub source for an API declaration.
*
* TODO: clintandrewhall - allow `base` to be overridden in the instance of a CI build
* associated with a PR.
*
* @param declaration - API declaration to generate link for.
* @returns GitHub link to the source code.
*/
function getLink(declaration: ApiDeclaration): string {
const base = `https://github.com/elastic/kibana/blob/main/${declaration.path}`;
if (declaration.lineNumber) {
return `${base}#L${declaration.lineNumber}`;
}
return `https://github.com/elastic/kibana/tree/main/${
declaration.path
}#:~:text=${encodeURIComponent(declaration.label)}`;
Expand Down Expand Up @@ -53,10 +60,38 @@ export function reportMetrics(
const { missingApiItems, referencedDeprecations } = apiMapResult;
const reporter = CiStatsReporter.fromEnv(log);

const printIssueTable = (title: string, rows: Array<{ id: string; link: string }>) => {
const count = rows.length;
if (count === 0) {
log.info(`${title}: none`);
return;
}
log.info(`${title} (${count})`);
// eslint-disable-next-line no-console
console.table(rows);
};

const printMissingExportsTable = (
title: string,
entries: Array<{ source: string; references: string }>
) => {
const header = title.toUpperCase();
const count = entries.length;
if (count === 0) {
log.info(`${header}: none`);
return;
}
log.info(`${header} (${count})`);
// eslint-disable-next-line no-console
console.table(
entries.map(({ source, references }) => ({
'Not exported source': source,
references,
}))
);
};

for (const plugin of plugins) {
// Note that the filtering is done here (per-plugin), rather than earlier in the pipeline.
// This keeps the metrics task aligned with how other docs tasks process plugins and ensures
// that all plugin data has been collected before selectively reporting metrics.
if (options.pluginFilter && !options.pluginFilter.includes(plugin.id)) {
continue;
}
Expand Down Expand Up @@ -162,7 +197,7 @@ export function reportMetrics(
// eslint-disable-next-line no-console
console.table(referencedDeprecations[id]);
} else {
log.info(`No referenced deprecations for plugin ${plugin.id}`);
log.info(`No referenced deprecations for ${plugin.id}`);
}
if (pluginStats.noReferences.length > 0) {
// eslint-disable-next-line no-console
Expand All @@ -173,7 +208,7 @@ export function reportMetrics(
}))
);
} else {
log.info(`No unused APIs for plugin ${plugin.id}`);
log.info(`No unused APIs for ${plugin.id}`);
}
}

Expand All @@ -184,25 +219,22 @@ export function reportMetrics(
pluginStats.deprecatedAPIsReferencedCount === 0 &&
(!missingApiItems[id] || Object.keys(missingApiItems[id]).length === 0);

log.info(`--- Plugin '${id}' ${passesAllChecks ? 'passes all checks ----' : '----'}`);
log.info(`--- '${id}' ${passesAllChecks ? 'passes all checks ----' : '----'}`);

if (!passesAllChecks) {
log.info(`${pluginStats.isAnyType.length} API items with ANY`);

if (options.stats.includes('any')) {
// eslint-disable-next-line no-console
console.table(
printIssueTable(
'API items with ANY',
pluginStats.isAnyType.map((d) => ({
id: d.id,
link: getLink(d),
}))
);
}

log.info(`${pluginStats.missingComments.length} API items missing comments`);
if (options.stats.includes('comments')) {
// eslint-disable-next-line no-console
console.table(
printIssueTable(
'API items missing comments',
pluginStats.missingComments.map((d) => ({
id: d.id,
link: getLink(d),
Expand All @@ -211,15 +243,12 @@ export function reportMetrics(
}

if (missingApiItems[id]) {
log.info(`${Object.keys(missingApiItems[id]).length} referenced API items not exported`);
if (options.stats.includes('exports')) {
// eslint-disable-next-line no-console
console.table(
Object.keys(missingApiItems[id]).map((key) => ({
'Not exported source': key,
references: missingApiItems[id][key].join(', '),
}))
);
const exportsTable = Object.keys(missingApiItems[id]).map((key) => ({
source: key,
references: missingApiItems[id][key].join(', '),
}));
printMissingExportsTable('Referenced API items not exported', exportsTable);
}
}
}
Expand Down
92 changes: 89 additions & 3 deletions packages/kbn-docs-utils/src/cli/tasks/setup_project.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import Path from 'path';
import { ToolingLog } from '@kbn/tooling-log';
import { setupProject } from './setup_project';
import type { CliContext, CliOptions } from '../types';
import type { FindPluginsOptions } from '../../find_plugins';

// Mock dependencies - order matters: mock get_all_doc_file_ids first to prevent globby from loading
jest.mock('../../mdx/get_all_doc_file_ids', () => ({
Expand Down Expand Up @@ -112,6 +113,7 @@ describe('setupProject', () => {
const result = await setupProject(context, options);

expect(result.plugins).toBeDefined();
expect(result.allPlugins).toBeDefined();
expect(result.pathsByPlugin).toBeDefined();
expect(result.project).toBeDefined();
});
Expand All @@ -134,9 +136,21 @@ describe('setupProject', () => {
const Fs = jest.requireMock('fs');
Fs.existsSync.mockReturnValue(true);

(findPlugins as jest.Mock).mockReturnValue([
{ id: 'test-plugin', isPlugin: true, directory: '/tmp/test' },
]);
const mockPlugin = {
id: 'test-plugin',
directory: '/mock/repo/root/src/plugins/test',
isPlugin: true,
manifest: { id: 'test-plugin', owner: { name: 'test-team' }, serviceFolders: [] },
manifestPath: '/mock/repo/root/src/plugins/test/kibana.json',
};

// Return all plugins for allPlugins, filtered for filteredPlugins
(findPlugins as jest.Mock).mockImplementation((options?: FindPluginsOptions) => {
if (options?.pluginFilter) {
return [mockPlugin];
}
return [mockPlugin, { ...mockPlugin, id: 'other-plugin' }];
});

const options: CliOptions = {
collectReferences: false,
Expand Down Expand Up @@ -173,4 +187,76 @@ describe('setupProject', () => {
"expected --package '@kbn/nonexistent-package' was not found"
);
});

it('scopes TypeScript project to single plugin directory when pluginFilter has one plugin', async () => {
const { Project } = jest.requireMock('ts-morph');
const mockProject = Project();

jest.clearAllMocks();

const mockPlugin = {
id: 'single-plugin',
directory: '/mock/repo/root/src/plugins/single',
isPlugin: true,
manifest: { id: 'single-plugin', owner: { name: 'test-team' }, serviceFolders: [] },
manifestPath: '/mock/repo/root/src/plugins/single/kibana.json',
};

// Return all plugins for allPlugins, filtered for plugins
(findPlugins as jest.Mock).mockImplementation((options?: FindPluginsOptions) => {
if (options?.pluginFilter) {
return [mockPlugin];
}
return [mockPlugin, { ...mockPlugin, id: 'other-plugin' }];
});

const options: CliOptions = {
collectReferences: false,
pluginFilter: ['single-plugin'],
};

await setupProject(context, options);

// Should use the single plugin's tsconfig
expect(Project).toHaveBeenCalledWith({
tsConfigFilePath: '/mock/repo/root/src/plugins/single/tsconfig.json',
skipAddingFilesFromTsConfig: true,
});

// Should only add files from the single plugin directory
expect(mockProject.addSourceFilesAtPaths).toHaveBeenCalledWith([
'/mock/repo/root/src/plugins/single/**/*.ts',
'!**/*.d.ts',
]);

// Should NOT call resolveSourceFileDependencies for single-plugin builds
expect(mockProject.resolveSourceFileDependencies).not.toHaveBeenCalled();
});

it('loads full codebase and resolves dependencies when no pluginFilter', async () => {
const { Project } = jest.requireMock('ts-morph');
const mockProject = Project();

jest.clearAllMocks();

(findPlugins as jest.Mock).mockReturnValue([]);

const options: CliOptions = {
collectReferences: false,
};

await setupProject(context, options);

// Should use the repo root tsconfig
expect(Project).toHaveBeenCalledWith({
tsConfigFilePath: '/mock/repo/root/tsconfig.json',
skipAddingFilesFromTsConfig: true,
});

// Should add files from all directories
expect(mockProject.addSourceFilesAtPaths).toHaveBeenCalledTimes(8);

// Should call resolveSourceFileDependencies for full builds
expect(mockProject.resolveSourceFileDependencies).toHaveBeenCalled();
});
});
Loading
Loading