Skip to content

Commit

Permalink
add tests about filtering and mapping plugin packages to PluginWrappers
Browse files Browse the repository at this point in the history
  • Loading branch information
spalger committed Jan 25, 2023
1 parent 39198ff commit 6e7fcff
Show file tree
Hide file tree
Showing 2 changed files with 109 additions and 16 deletions.
104 changes: 97 additions & 7 deletions packages/core/plugins/core-plugins-server-internal/src/discovery/plugins_discovery.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { mockPackage, scanPluginSearchPathsMock } from './plugins_discovery.test
import mockFs from 'mock-fs';
import { getEnvOptions, rawConfigServiceMock } from '@kbn/config-mocks';
import { loggingSystemMock } from '@kbn/core-logging-server-mocks';
import type { Package } from '@kbn/repo-packages';

import { firstValueFrom, from } from 'rxjs';
import { map, toArray } from 'rxjs/operators';
Expand All @@ -25,6 +26,34 @@ import { discover } from './plugins_discovery';
import { PluginType } from '@kbn/core-base-common';

const KIBANA_ROOT = process.cwd();
jest.mock('@kbn/repo-packages', () => ({
...jest.requireActual('@kbn/repo-packages'),
getPluginPackagesFilter: jest.fn().mockReturnValue(() => true),
}));

jest.mock('./plugin_manifest_from_plugin_package', () => ({
pluginManifestFromPluginPackage: jest.fn((version, pkgManifest) => ({
version,
...pkgManifest,
})),
}));

const getPluginPackagesFilterMock: jest.Mock =
jest.requireMock('@kbn/repo-packages').getPluginPackagesFilter;
const pluginManifestFromPluginPackageMock: jest.Mock = jest.requireMock(
'./plugin_manifest_from_plugin_package'
).pluginManifestFromPluginPackage;

function getMockPackage(id: string) {
return {
id,
manifest: {
id,
type: 'plugin',
},
directory: resolve(REPO_ROOT, `packages/${id}`),
} as Package;
}

const Plugins = {
invalid: () => ({
Expand Down Expand Up @@ -180,6 +209,8 @@ describe('plugins discovery system', () => {
jest.spyOn(console, 'log').mockImplementation((...args) => {
process.stdout.write(args + '\n');
});

jest.clearAllMocks();
});

afterEach(() => {
Expand Down Expand Up @@ -542,9 +573,60 @@ describe('plugins discovery system', () => {
expect(loggingSystemMock.collect(logger).warn).toEqual([]);
});

describe('plugin packages', () => {
it('filters repoPackages in the env and converts them to PluginWrappers', async () => {
const foo = getMockPackage('foo');
const bar = getMockPackage('bar');
coreContext.env = {
...env,
repoPackages: [foo, bar],
};
const filterFn = jest.fn((p: Package) => p === foo);
getPluginPackagesFilterMock.mockReturnValue(filterFn);

const { plugin$ } = discover({
config: new PluginsConfig(pluginConfig, coreContext.env),
coreContext,
instanceInfo,
nodeInfo,
});

const [plugin, ...empty] = await firstValueFrom(plugin$.pipe(toArray()));
expect(empty).toHaveLength(0);

expect(getPluginPackagesFilterMock).toHaveBeenCalledTimes(1);
const filterArgs = getPluginPackagesFilterMock.mock.calls[0];
expect(filterArgs).toEqual([
{
examples: false,
oss: false,
parentDirs: [pluginDir()],
paths: [],
},
]);

expect(filterFn).toHaveBeenCalledTimes(2);
expect(filterFn.mock.calls[0]).toEqual([foo, 0]);
expect(filterFn.mock.calls[1]).toEqual([bar, 1]);
expect(filterFn.mock.results).toEqual([
{ type: 'return', value: true },
{ type: 'return', value: false },
]);

expect(pluginManifestFromPluginPackageMock).toHaveBeenCalledTimes(1);
const manifestArgs = pluginManifestFromPluginPackageMock.mock.calls[0];
expect(manifestArgs).toEqual([coreContext.env.packageInfo.version, foo.manifest]);
expect(pluginManifestFromPluginPackageMock.mock.results[0]).toEqual({
type: 'return',
value: plugin.manifest,
});
});
});

describe('discovery order', () => {
beforeEach(() => {
scanPluginSearchPathsMock.mockClear();
getPluginPackagesFilterMock.mockReturnValue(() => true);
});

it('returns the plugins in a deterministic order', async () => {
Expand All @@ -565,8 +647,13 @@ describe('plugins discovery system', () => {
])
);

coreContext.env = {
...env,
repoPackages: [getMockPackage('foo'), getMockPackage('bar')],
};

let { plugin$ } = discover({
config: new PluginsConfig(pluginConfig, env),
config: new PluginsConfig(pluginConfig, coreContext.env),
coreContext,
instanceInfo,
nodeInfo,
Expand All @@ -576,9 +663,8 @@ describe('plugins discovery system', () => {
let plugins = await firstValueFrom(plugin$.pipe(toArray()));
let pluginNames = plugins.map((plugin) => plugin.name);

expect(pluginNames).toHaveLength(3);
// order coming from `ROOT/plugin` -> `ROOT/src/plugins` -> // ROOT/x-pack
expect(pluginNames).toEqual(['pluginB', 'pluginA', 'pluginC']);
// order coming from `ROOT/packages` -> `ROOT/plugin` -> `ROOT/src/plugins` -> // ROOT/x-pack
expect(pluginNames).toEqual(['bar', 'foo', 'pluginB', 'pluginA', 'pluginC']);

// second pass
scanPluginSearchPathsMock.mockReturnValue(
Expand All @@ -589,6 +675,11 @@ describe('plugins discovery system', () => {
])
);

coreContext.env = {
...env,
repoPackages: [getMockPackage('bar'), getMockPackage('foo')],
};

plugin$ = discover({
config: new PluginsConfig(pluginConfig, env),
coreContext,
Expand All @@ -600,9 +691,8 @@ describe('plugins discovery system', () => {
plugins = await firstValueFrom(plugin$.pipe(toArray()));
pluginNames = plugins.map((plugin) => plugin.name);

expect(pluginNames).toHaveLength(3);
// order coming from `ROOT/plugin` -> `ROOT/src/plugins` -> // ROOT/x-pack
expect(pluginNames).toEqual(['pluginB', 'pluginA', 'pluginC']);
// order coming from `ROOT/packages` -> `ROOT/plugin` -> `ROOT/src/plugins` -> // ROOT/x-pack
expect(pluginNames).toEqual(['bar', 'foo', 'pluginB', 'pluginA', 'pluginC']);
});
});
});
21 changes: 12 additions & 9 deletions packages/core/plugins/core-plugins-server-internal/src/discovery/plugins_discovery.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,14 +54,6 @@ export function discover({
from(config.additionalPluginPaths),
scanPluginSearchPaths(config.pluginSearchPaths, log)
).pipe(
toArray(),
mergeMap((pathAndErrors) => {
return pathAndErrors.sort((a, b) => {
const pa = typeof a === 'string' ? a : a.path;
const pb = typeof b === 'string' ? b : b.path;
return pa < pb ? -1 : pa > pb ? 1 : 0;
});
}),
concatMap((pluginPathOrError) => {
return typeof pluginPathOrError === 'string'
? createPlugin$(pluginPathOrError, log, coreContext, instanceInfo, nodeInfo)
Expand Down Expand Up @@ -101,7 +93,18 @@ export function discover({
})
);

const discoveryResults$ = merge(fsDiscovery$, pluginPkgDiscovery$).pipe(shareReplay());
const discoveryResults$ = merge(fsDiscovery$, pluginPkgDiscovery$).pipe(
toArray(),
// ensure that everything is always provided in a consistent order
mergeMap((pkgs) =>
pkgs.sort((a, b) => {
const aComp = typeof a !== 'string' ? a.path : a;
const bComp = typeof b !== 'string' ? b.path : b;
return aComp.localeCompare(bComp);
})
),
shareReplay()
);

return {
plugin$: discoveryResults$.pipe(
Expand Down

0 comments on commit 6e7fcff

Please sign in to comment.