From 54c5836f7a534011d20f631e389249ce9420d0f3 Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Fri, 25 Nov 2022 14:32:05 +0100 Subject: [PATCH 1/2] Deprecate withSubject() in favor of type-specific methods Signed-off-by: Eric Peterson --- .changeset/search-tofu-panaeng.md | 5 ++ plugins/search-backend-node/api-report.md | 7 ++ .../src/test-utils/TestPipeline.ts | 77 +++++++++++++++++++ 3 files changed, 89 insertions(+) create mode 100644 .changeset/search-tofu-panaeng.md diff --git a/.changeset/search-tofu-panaeng.md b/.changeset/search-tofu-panaeng.md new file mode 100644 index 0000000000..1f8bbf6a7b --- /dev/null +++ b/.changeset/search-tofu-panaeng.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-search-backend-node': patch +--- + +Use of `TestPipeline.withSubject()` is now deprecated. Instead, use the `fromCollator`, `fromDecorator`, or `fromIndexer` static methods to instantiate a test pipeline. You may also use the class' `withCollator`, `withDecorator`, and `withIndexer` instance methods to build test pipelines that consist of multiple test subjects. diff --git a/plugins/search-backend-node/api-report.md b/plugins/search-backend-node/api-report.md index 62a4eabc26..734bf6e1c0 100644 --- a/plugins/search-backend-node/api-report.md +++ b/plugins/search-backend-node/api-report.md @@ -173,7 +173,14 @@ export type ScheduleTaskParameters = { // @public export class TestPipeline { execute(): Promise; + static fromCollator(collator: Readable): TestPipeline; + static fromDecorator(decorator: Transform): TestPipeline; + static fromIndexer(indexer: Writable): TestPipeline; + withCollator(collator: Readable): this; + withDecorator(decorator: Transform): this; withDocuments(documents: IndexableDocument[]): TestPipeline; + withIndexer(indexer: Writable): this; + // @deprecated static withSubject(subject: Readable | Transform | Writable): TestPipeline; } diff --git a/plugins/search-backend-node/src/test-utils/TestPipeline.ts b/plugins/search-backend-node/src/test-utils/TestPipeline.ts index 00dd9b3755..33097daae9 100644 --- a/plugins/search-backend-node/src/test-utils/TestPipeline.ts +++ b/plugins/search-backend-node/src/test-utils/TestPipeline.ts @@ -37,6 +37,35 @@ export type TestPipelineResult = { /** * Test utility for Backstage Search collators, decorators, and indexers. + * + * @example + * An example test checking that a collator provides expected documents. + * ``` + * it('provides expected documents', async () => { + * const testSubject = await yourCollatorFactory.getCollator(); + * const pipeline = TestPipeline.fromCollator(testSubject); + * + * const { documents } = await pipeline.execute(); + * + * expect(documents).toHaveLength(2); + * }) + * ``` + * + * @example + * An example test checking that a decorator behaves as expected. + * ``` + * it('filters private documents', async () => { + * const testSubject = await yourDecoratorFactory.getDecorator(); + * const pipeline = TestPipeline + * .fromDecorator(testSubject) + * .withDocuments([{ title: 'Private', location: '/private', text: '' }]); + * + * const { documents } = await pipeline.execute(); + * + * expect(documents).toHaveLength(0); + * }) + * ``` + * * @public */ export class TestPipeline { @@ -60,6 +89,9 @@ export class TestPipeline { /** * Provide the collator, decorator, or indexer to be tested. + * + * @deprecated Use `fromCollator`, `fromDecorator` or `fromIndexer` static + * methods to create a test pipeline instead. */ static withSubject(subject: Readable | Transform | Writable) { if (subject instanceof Transform) { @@ -79,6 +111,51 @@ export class TestPipeline { ); } + /** + * Create a test pipeline given a collator you want to test. + */ + static fromCollator(collator: Readable) { + return new TestPipeline({ collator }); + } + + /** + * Add a collator to the test pipeline. + */ + withCollator(collator: Readable): this { + this.collator = collator; + return this; + } + + /** + * Create a test pipeline given a decorator you want to test. + */ + static fromDecorator(decorator: Transform) { + return new TestPipeline({ decorator }); + } + + /** + * Add a decorator to the test pipeline. + */ + withDecorator(decorator: Transform): this { + this.decorator = decorator; + return this; + } + + /** + * Create a test pipeline given an indexer you want to test. + */ + static fromIndexer(indexer: Writable) { + return new TestPipeline({ indexer }); + } + + /** + * Add an indexer to the test pipeline. + */ + withIndexer(indexer: Writable): this { + this.indexer = indexer; + return this; + } + /** * Provide documents for testing decorators and indexers. */ From aff7351fa5b7a16b76cebd24a3ee187dfe63f94d Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Fri, 25 Nov 2022 14:33:23 +0100 Subject: [PATCH 2/2] Use non-deprecated TestPipeline static methods Signed-off-by: Eric Peterson --- .../DefaultCatalogCollatorFactory.test.ts | 10 ++--- .../ElasticSearchSearchEngineIndexer.test.ts | 6 +-- .../PgSearchEngineIndexer.test.ts | 10 ++--- ...ewlineDelimitedJsonCollatorFactory.test.ts | 2 +- .../src/engines/LunrSearchEngine.test.ts | 38 +++++++++---------- .../engines/LunrSearchEngineIndexer.test.ts | 6 +-- .../indexing/BatchSearchEngineIndexer.test.ts | 2 +- .../src/indexing/DecoratorBase.test.ts | 6 +-- .../DefaultTechDocsCollatorFactory.test.ts | 10 ++--- 9 files changed, 45 insertions(+), 45 deletions(-) diff --git a/plugins/catalog-backend/src/search/DefaultCatalogCollatorFactory.test.ts b/plugins/catalog-backend/src/search/DefaultCatalogCollatorFactory.test.ts index 5fc8ca6fbf..8b4e5601b4 100644 --- a/plugins/catalog-backend/src/search/DefaultCatalogCollatorFactory.test.ts +++ b/plugins/catalog-backend/src/search/DefaultCatalogCollatorFactory.test.ts @@ -114,14 +114,14 @@ describe('DefaultCatalogCollatorFactory', () => { }); it('fetches from the configured catalog service', async () => { - const pipeline = TestPipeline.withSubject(collator); + const pipeline = TestPipeline.fromCollator(collator); const { documents } = await pipeline.execute(); expect(mockDiscoveryApi.getBaseUrl).toHaveBeenCalledWith('catalog'); expect(documents).toHaveLength(expectedEntities.length); }); it('maps a returned entity to an expected CatalogEntityDocument', async () => { - const pipeline = TestPipeline.withSubject(collator); + const pipeline = TestPipeline.fromCollator(collator); const { documents } = await pipeline.execute(); expect(documents[0]).toMatchObject({ @@ -159,7 +159,7 @@ describe('DefaultCatalogCollatorFactory', () => { }); collator = await factory.getCollator(); - const pipeline = TestPipeline.withSubject(collator); + const pipeline = TestPipeline.fromCollator(collator); const { documents } = await pipeline.execute(); expect(documents[0]).toMatchObject({ location: '/software/test-entity', @@ -177,7 +177,7 @@ describe('DefaultCatalogCollatorFactory', () => { }); collator = await factory.getCollator(); - const pipeline = TestPipeline.withSubject(collator); + const pipeline = TestPipeline.fromCollator(collator); const { documents } = await pipeline.execute(); // The simulated 'Foo,Bar' filter should return in an empty list @@ -191,7 +191,7 @@ describe('DefaultCatalogCollatorFactory', () => { }); collator = await factory.getCollator(); - const pipeline = TestPipeline.withSubject(collator); + const pipeline = TestPipeline.fromCollator(collator); const { documents } = await pipeline.execute(); expect(documents).toHaveLength(expectedEntities.length); diff --git a/plugins/search-backend-module-elasticsearch/src/engines/ElasticSearchSearchEngineIndexer.test.ts b/plugins/search-backend-module-elasticsearch/src/engines/ElasticSearchSearchEngineIndexer.test.ts index a19687a03b..155a6bbf32 100644 --- a/plugins/search-backend-module-elasticsearch/src/engines/ElasticSearchSearchEngineIndexer.test.ts +++ b/plugins/search-backend-module-elasticsearch/src/engines/ElasticSearchSearchEngineIndexer.test.ts @@ -137,7 +137,7 @@ describe('ElasticSearchSearchEngineIndexer', () => { }, ]; - await TestPipeline.withSubject(indexer).withDocuments(documents).execute(); + await TestPipeline.fromIndexer(indexer).withDocuments(documents).execute(); // Older indices should have been queried for. expect(catSpy).toHaveBeenCalled(); @@ -184,7 +184,7 @@ describe('ElasticSearchSearchEngineIndexer', () => { text: range(2000).join(', '), })); - await TestPipeline.withSubject(indexer).withDocuments(documents).execute(); + await TestPipeline.fromIndexer(indexer).withDocuments(documents).execute(); // Ensure multiple bulk requests were made. expect(bulkSpy).toHaveBeenCalledTimes(2); @@ -221,7 +221,7 @@ describe('ElasticSearchSearchEngineIndexer', () => { catSpy, ); - await TestPipeline.withSubject(indexer).withDocuments(documents).execute(); + await TestPipeline.fromIndexer(indexer).withDocuments(documents).execute(); // Final deletion shouldn't be called. expect(deleteSpy).not.toHaveBeenCalled(); diff --git a/plugins/search-backend-module-pg/src/PgSearchEngine/PgSearchEngineIndexer.test.ts b/plugins/search-backend-module-pg/src/PgSearchEngine/PgSearchEngineIndexer.test.ts index 03b9774751..0b9cd96c19 100644 --- a/plugins/search-backend-module-pg/src/PgSearchEngine/PgSearchEngineIndexer.test.ts +++ b/plugins/search-backend-module-pg/src/PgSearchEngine/PgSearchEngineIndexer.test.ts @@ -53,7 +53,7 @@ describe('PgSearchEngineIndexer', () => { }, ]; - await TestPipeline.withSubject(indexer).withDocuments(documents).execute(); + await TestPipeline.fromIndexer(indexer).withDocuments(documents).execute(); expect(database.getTransaction).toHaveBeenCalledTimes(1); expect(database.prepareInsert).toHaveBeenCalledTimes(1); @@ -73,7 +73,7 @@ describe('PgSearchEngineIndexer', () => { location: `location-${i}`, })); - await TestPipeline.withSubject(indexer).withDocuments(documents).execute(); + await TestPipeline.fromIndexer(indexer).withDocuments(documents).execute(); expect(database.getTransaction).toHaveBeenCalledTimes(1); expect(database.prepareInsert).toHaveBeenCalledTimes(1); @@ -92,7 +92,7 @@ describe('PgSearchEngineIndexer', () => { ]; database.prepareInsert.mockRejectedValueOnce(expectedError); - const result = await TestPipeline.withSubject(indexer) + const result = await TestPipeline.fromIndexer(indexer) .withDocuments(documents) .execute(); @@ -114,7 +114,7 @@ describe('PgSearchEngineIndexer', () => { ]; database.insertDocuments.mockRejectedValueOnce(expectedError); - const result = await TestPipeline.withSubject(indexer) + const result = await TestPipeline.fromIndexer(indexer) .withDocuments(documents) .execute(); @@ -136,7 +136,7 @@ describe('PgSearchEngineIndexer', () => { ]; database.completeInsert.mockRejectedValueOnce(expectedError); - const result = await TestPipeline.withSubject(indexer) + const result = await TestPipeline.fromIndexer(indexer) .withDocuments(documents) .execute(); diff --git a/plugins/search-backend-node/src/collators/NewlineDelimitedJsonCollatorFactory.test.ts b/plugins/search-backend-node/src/collators/NewlineDelimitedJsonCollatorFactory.test.ts index 7120fc7ce5..c08db8f28e 100644 --- a/plugins/search-backend-node/src/collators/NewlineDelimitedJsonCollatorFactory.test.ts +++ b/plugins/search-backend-node/src/collators/NewlineDelimitedJsonCollatorFactory.test.ts @@ -142,7 +142,7 @@ describe('DefaultCatalogCollatorFactory', () => { }); const collator = await factory.getCollator(); - const pipeline = TestPipeline.withSubject(collator); + const pipeline = TestPipeline.fromCollator(collator); const { documents } = await pipeline.execute(); expect(documents).toHaveLength(2); diff --git a/plugins/search-backend-node/src/engines/LunrSearchEngine.test.ts b/plugins/search-backend-node/src/engines/LunrSearchEngine.test.ts index f0518cd71d..4ef380943f 100644 --- a/plugins/search-backend-node/src/engines/LunrSearchEngine.test.ts +++ b/plugins/search-backend-node/src/engines/LunrSearchEngine.test.ts @@ -381,7 +381,7 @@ describe('LunrSearchEngine', () => { testLunrSearchEngine, 'test-index', ); - await TestPipeline.withSubject(indexer) + await TestPipeline.fromIndexer(indexer) .withDocuments(mockDocuments) .execute(); @@ -412,7 +412,7 @@ describe('LunrSearchEngine', () => { testLunrSearchEngine, 'test-index', ); - await TestPipeline.withSubject(indexer) + await TestPipeline.fromIndexer(indexer) .withDocuments(mockDocuments) .execute(); @@ -452,7 +452,7 @@ describe('LunrSearchEngine', () => { testLunrSearchEngine, 'test-index', ); - await TestPipeline.withSubject(indexer) + await TestPipeline.fromIndexer(indexer) .withDocuments(mockDocuments) .execute(); @@ -495,7 +495,7 @@ describe('LunrSearchEngine', () => { inspectableSearchEngine, 'test-index', ); - await TestPipeline.withSubject(indexer) + await TestPipeline.fromIndexer(indexer) .withDocuments(mockDocuments) .execute(); @@ -544,7 +544,7 @@ describe('LunrSearchEngine', () => { testLunrSearchEngine, 'test-index', ); - await TestPipeline.withSubject(indexer) + await TestPipeline.fromIndexer(indexer) .withDocuments(mockDocuments) .execute(); @@ -583,7 +583,7 @@ describe('LunrSearchEngine', () => { testLunrSearchEngine, 'test-index', ); - await TestPipeline.withSubject(indexer) + await TestPipeline.fromIndexer(indexer) .withDocuments(mockDocuments) .execute(); @@ -623,7 +623,7 @@ describe('LunrSearchEngine', () => { testLunrSearchEngine, 'test-index', ); - await TestPipeline.withSubject(indexer) + await TestPipeline.fromIndexer(indexer) .withDocuments(mockDocuments) .execute(); @@ -663,7 +663,7 @@ describe('LunrSearchEngine', () => { testLunrSearchEngine, 'test-index', ); - await TestPipeline.withSubject(indexer) + await TestPipeline.fromIndexer(indexer) .withDocuments(mockDocuments) .execute(); @@ -708,7 +708,7 @@ describe('LunrSearchEngine', () => { testLunrSearchEngine, 'test-index', ); - await TestPipeline.withSubject(indexer) + await TestPipeline.fromIndexer(indexer) .withDocuments(mockDocuments) .execute(); @@ -763,10 +763,10 @@ describe('LunrSearchEngine', () => { testLunrSearchEngine, 'test-index-2', ); - await TestPipeline.withSubject(indexer1) + await TestPipeline.fromIndexer(indexer1) .withDocuments(mockDocuments) .execute(); - await TestPipeline.withSubject(indexer2) + await TestPipeline.fromIndexer(indexer2) .withDocuments(mockDocuments2) .execute(); @@ -811,7 +811,7 @@ describe('LunrSearchEngine', () => { testLunrSearchEngine, 'test-index', ); - await TestPipeline.withSubject(indexer) + await TestPipeline.fromIndexer(indexer) .withDocuments(mockDocuments) .execute(); @@ -871,14 +871,14 @@ describe('LunrSearchEngine', () => { testLunrSearchEngine, 'test-index', ); - await TestPipeline.withSubject(indexer) + await TestPipeline.fromIndexer(indexer) .withDocuments(mockDocuments) .execute(); const indexer2 = await getActualIndexer( testLunrSearchEngine, 'test-index-2', ); - await TestPipeline.withSubject(indexer2) + await TestPipeline.fromIndexer(indexer2) .withDocuments(mockDocuments2) .execute(); @@ -924,7 +924,7 @@ describe('LunrSearchEngine', () => { testLunrSearchEngine, 'test-index', ); - await TestPipeline.withSubject(indexer) + await TestPipeline.fromIndexer(indexer) .withDocuments(mockDocuments) .execute(); @@ -974,7 +974,7 @@ describe('LunrSearchEngine', () => { })); const indexer = await getActualIndexer(testLunrSearchEngine, 'test-index'); - await TestPipeline.withSubject(indexer) + await TestPipeline.fromIndexer(indexer) .withDocuments(mockDocuments) .execute(); @@ -1112,7 +1112,7 @@ describe('stopword testing', () => { const indexer = await getActualIndexer(testLunrSearchEngine, 'test-index'); - await TestPipeline.withSubject(indexer) + await TestPipeline.fromIndexer(indexer) .withDocuments(mockDocuments) .execute(); @@ -1149,7 +1149,7 @@ describe('stopword testing', () => { const indexer = await getActualIndexer(testLunrSearchEngine, 'test-index'); - await TestPipeline.withSubject(indexer) + await TestPipeline.fromIndexer(indexer) .withDocuments(mockDocuments) .execute(); @@ -1186,7 +1186,7 @@ describe('stopword testing', () => { const indexer = await getActualIndexer(testLunrSearchEngine, 'test-index'); - await TestPipeline.withSubject(indexer) + await TestPipeline.fromIndexer(indexer) .withDocuments(mockDocuments) .execute(); diff --git a/plugins/search-backend-node/src/engines/LunrSearchEngineIndexer.test.ts b/plugins/search-backend-node/src/engines/LunrSearchEngineIndexer.test.ts index fbb6b153d0..ff76f0fa7c 100644 --- a/plugins/search-backend-node/src/engines/LunrSearchEngineIndexer.test.ts +++ b/plugins/search-backend-node/src/engines/LunrSearchEngineIndexer.test.ts @@ -58,7 +58,7 @@ describe('LunrSearchEngineIndexer', () => { }, ]; - await TestPipeline.withSubject(indexer).withDocuments(documents).execute(); + await TestPipeline.fromIndexer(indexer).withDocuments(documents).execute(); expect(lunrBuilderAddSpy).toHaveBeenCalledWith(documents[0]); }); @@ -70,7 +70,7 @@ describe('LunrSearchEngineIndexer', () => { location: `location-${i}`, })); - await TestPipeline.withSubject(indexer).withDocuments(documents).execute(); + await TestPipeline.fromIndexer(indexer).withDocuments(documents).execute(); expect(lunrBuilderAddSpy).toHaveBeenCalledTimes(350); }); @@ -84,7 +84,7 @@ describe('LunrSearchEngineIndexer', () => { }, ]; - await TestPipeline.withSubject(indexer).withDocuments(documents).execute(); + await TestPipeline.fromIndexer(indexer).withDocuments(documents).execute(); // Builder ref should be set to location (and only once). expect(lunrBuilderRefSpy).toHaveBeenCalledTimes(1); diff --git a/plugins/search-backend-node/src/indexing/BatchSearchEngineIndexer.test.ts b/plugins/search-backend-node/src/indexing/BatchSearchEngineIndexer.test.ts index b4c9ff9e46..a51a479e7e 100644 --- a/plugins/search-backend-node/src/indexing/BatchSearchEngineIndexer.test.ts +++ b/plugins/search-backend-node/src/indexing/BatchSearchEngineIndexer.test.ts @@ -46,7 +46,7 @@ describe('BatchSearchEngineIndexer', () => { it('should work end-to-end', async () => { const indexer = new ConcreteBatchIndexer({ batchSize: 1 }); - await TestPipeline.withSubject(indexer) + await TestPipeline.fromIndexer(indexer) .withDocuments([document, document, document]) .execute(); expect(indexSpy).toHaveBeenCalledTimes(3); diff --git a/plugins/search-backend-node/src/indexing/DecoratorBase.test.ts b/plugins/search-backend-node/src/indexing/DecoratorBase.test.ts index 12c61bd7d3..875ef11940 100644 --- a/plugins/search-backend-node/src/indexing/DecoratorBase.test.ts +++ b/plugins/search-backend-node/src/indexing/DecoratorBase.test.ts @@ -54,7 +54,7 @@ describe('DecoratorBase', () => { })); const decorator = new ConcreteDecorator(); - const { documents } = await TestPipeline.withSubject(decorator) + const { documents } = await TestPipeline.fromDecorator(decorator) .withDocuments([document, document, document]) .execute(); @@ -68,7 +68,7 @@ describe('DecoratorBase', () => { decorateSpy.mockResolvedValue(undefined); const decorator = new ConcreteDecorator(); - const { documents } = await TestPipeline.withSubject(decorator) + const { documents } = await TestPipeline.fromDecorator(decorator) .withDocuments([document, document, document]) .execute(); @@ -82,7 +82,7 @@ describe('DecoratorBase', () => { }); const decorator = new ConcreteDecorator(); - const { documents } = await TestPipeline.withSubject(decorator) + const { documents } = await TestPipeline.fromDecorator(decorator) .withDocuments([document, document, document]) .execute(); diff --git a/plugins/techdocs-backend/src/search/DefaultTechDocsCollatorFactory.test.ts b/plugins/techdocs-backend/src/search/DefaultTechDocsCollatorFactory.test.ts index 659f4783ce..d9329151c6 100644 --- a/plugins/techdocs-backend/src/search/DefaultTechDocsCollatorFactory.test.ts +++ b/plugins/techdocs-backend/src/search/DefaultTechDocsCollatorFactory.test.ts @@ -145,7 +145,7 @@ describe('DefaultTechDocsCollatorFactory', () => { }); it('fetches from the configured catalog and tech docs services', async () => { - const pipeline = TestPipeline.withSubject(collator); + const pipeline = TestPipeline.fromCollator(collator); const { documents } = await pipeline.execute(); expect(mockDiscoveryApi.getBaseUrl).toHaveBeenCalledWith('catalog'); expect(mockDiscoveryApi.getBaseUrl).toHaveBeenCalledWith('techdocs'); @@ -153,7 +153,7 @@ describe('DefaultTechDocsCollatorFactory', () => { }); it('should create documents for each tech docs search index', async () => { - const pipeline = TestPipeline.withSubject(collator); + const pipeline = TestPipeline.fromCollator(collator); const { documents } = await pipeline.execute(); const entity = expectedEntities[0]; documents.forEach((document, idx) => { @@ -182,7 +182,7 @@ describe('DefaultTechDocsCollatorFactory', () => { }); collator = await factory.getCollator(); - const pipeline = TestPipeline.withSubject(collator); + const pipeline = TestPipeline.fromCollator(collator); const { documents } = await pipeline.execute(); expect(documents[0]).toMatchObject({ @@ -200,7 +200,7 @@ describe('DefaultTechDocsCollatorFactory', () => { }); collator = await factory.getCollator(); - const pipeline = TestPipeline.withSubject(collator); + const pipeline = TestPipeline.fromCollator(collator); const { documents } = await pipeline.execute(); // Only 1 entity with TechDocs configured multiplied by 3 pages. @@ -222,7 +222,7 @@ describe('DefaultTechDocsCollatorFactory', () => { }); it('should create documents for each tech docs search index', async () => { - const pipeline = TestPipeline.withSubject(collator); + const pipeline = TestPipeline.fromCollator(collator); const { documents } = await pipeline.execute(); const entity = expectedEntities[0]; documents.forEach((document, idx) => {