From 6776e630c3f0461b30e68280b4cf452e91ddaba4 Mon Sep 17 00:00:00 2001 From: kaizen3031593 Date: Thu, 23 Dec 2021 14:32:44 -0500 Subject: [PATCH 1/3] fix(rosetta): infused snippets not returned from cache --- packages/jsii-rosetta/bin/jsii-rosetta.ts | 26 ++++++++++++++++--- .../jsii-rosetta/lib/rosetta-translator.ts | 11 ++++++++ .../test/commands/extract.test.ts | 5 ++-- 3 files changed, 36 insertions(+), 6 deletions(-) diff --git a/packages/jsii-rosetta/bin/jsii-rosetta.ts b/packages/jsii-rosetta/bin/jsii-rosetta.ts index a3e059f18b..20ef8c46c5 100644 --- a/packages/jsii-rosetta/bin/jsii-rosetta.ts +++ b/packages/jsii-rosetta/bin/jsii-rosetta.ts @@ -82,19 +82,39 @@ function main() { describe: 'Output file to store logging results. Ignored if -log is not true', default: DEFAULT_INFUSION_RESULTS_NAME, }) + .option('cache-from', { + alias: 'C', + type: 'string', + // eslint-disable-next-line prettier/prettier + describe: + 'Reuse translations from the given tablet file if the snippet and type definitions did not change', + requiresArg: true, + default: undefined, + }) .option('cache-to', { alias: 'o', type: 'string', describe: 'Append all translated snippets to the given tablet file', requiresArg: true, default: undefined, - }), + }) + .option('cache', { + alias: 'k', + type: 'string', + describe: 'Alias for --cache-from and --cache-to together', + requiresArg: true, + default: undefined, + }) + .conflicts('cache', 'cache-from') + .conflicts('cache', 'cache-to'), wrapHandler(async (args) => { const absAssemblies = (args.ASSEMBLY.length > 0 ? args.ASSEMBLY : ['.']).map((x) => path.resolve(x)); - const cacheToFile = fmap(args['cache-to'], path.resolve); + const absCacheFrom = fmap(args.cache ?? args['cache-from'], path.resolve); + const absCacheTo = fmap(args.cache ?? args['cache-to'], path.resolve); const result = await infuse(absAssemblies, { logFile: args['log-file'], - cacheToFile: cacheToFile, + cacheToFile: absCacheTo, + cacheFromFile: absCacheFrom, }); let totalTypes = 0; diff --git a/packages/jsii-rosetta/lib/rosetta-translator.ts b/packages/jsii-rosetta/lib/rosetta-translator.ts index e9e84a3c4f..1a00e463c6 100644 --- a/packages/jsii-rosetta/lib/rosetta-translator.ts +++ b/packages/jsii-rosetta/lib/rosetta-translator.ts @@ -149,6 +149,13 @@ export class RosettaTranslator { function tryReadFromCache(sourceSnippet: TypeScriptSnippet, cache: LanguageTablet, fingerprinter: TypeFingerprinter) { const fromCache = cache.tryGetSnippet(snippetKey(sourceSnippet)); + // infused snippets won't pass the full source check or the fingerprinter + // but there is no reason to try to recompile it, so return cached snippet + // if there exists one. + if (isInfused(sourceSnippet)) { + return fromCache; + } + const cacheable = fromCache && completeSource(sourceSnippet) === fromCache.snippet.fullSource && @@ -160,6 +167,10 @@ function tryReadFromCache(sourceSnippet: TypeScriptSnippet, cache: LanguageTable return cacheable ? fromCache : undefined; } +function isInfused(snippet: TypeScriptSnippet) { + return snippet.parameters?.infused !== undefined; +} + export interface ReadFromCacheResults { readonly translations: TranslatedSnippet[]; readonly remaining: TypeScriptSnippet[]; diff --git a/packages/jsii-rosetta/test/commands/extract.test.ts b/packages/jsii-rosetta/test/commands/extract.test.ts index 74ff8da273..c6fe94b545 100644 --- a/packages/jsii-rosetta/test/commands/extract.test.ts +++ b/packages/jsii-rosetta/test/commands/extract.test.ts @@ -456,7 +456,7 @@ test('infused examples skip loose mode', async () => { try { const cacheToFile = path.join(otherAssembly.moduleDirectory, 'test.tabl.json'); - // Without exampleMetadata infused=true, expect an error + // Without exampleMetadata infused, expect an error await expect( extract.extractSnippets([otherAssembly.moduleDirectory], { cacheToFile, @@ -465,8 +465,7 @@ test('infused examples skip loose mode', async () => { ).rejects.toThrowError(/Sample uses literate source/); // Add infused=true to metadata and update assembly - otherAssembly.assembly.types!['my_assembly.ClassA'].docs!.custom!.exampleMetadata = - 'lit=integ.test.ts infused=true'; + otherAssembly.assembly.types!['my_assembly.ClassA'].docs!.custom!.exampleMetadata = 'lit=integ.test.ts infused'; await otherAssembly.updateAssembly(); // Expect same function call to succeed now From c9a53c7f843a75905562236be48521b33332bd56 Mon Sep 17 00:00:00 2001 From: kaizen3031593 Date: Thu, 23 Dec 2021 15:40:34 -0500 Subject: [PATCH 2/3] add test --- .../test/commands/extract.test.ts | 52 ++++++++++++++++++- 1 file changed, 51 insertions(+), 1 deletion(-) diff --git a/packages/jsii-rosetta/test/commands/extract.test.ts b/packages/jsii-rosetta/test/commands/extract.test.ts index c6fe94b545..016b4eaaf6 100644 --- a/packages/jsii-rosetta/test/commands/extract.test.ts +++ b/packages/jsii-rosetta/test/commands/extract.test.ts @@ -432,6 +432,56 @@ test('extract and infuse in one command', async () => { expect(types!['my_assembly.ClassA'].docs?.example).toBeDefined(); }); +test('infused examples always returned from cache', async () => { + const otherAssembly = await TestJsiiModule.fromSource( + { + 'index.ts': ` + /** + * ClassA + * + * @exampleMetadata infused + * @example x + */ + export class ClassA { + public someMethod() { + } + } + `, + }, + { + name: 'my_assembly', + jsii: DUMMY_JSII_CONFIG, + }, + ); + try { + const cacheFile = path.join(otherAssembly.moduleDirectory, 'test.tabl.json'); + + // Cache to file + await extract.extractSnippets([otherAssembly.moduleDirectory], { + cacheToFile: cacheFile, + ...defaultExtractOptions, + }); + + // Update the example with a fixture that would fail compilation + otherAssembly.assembly.types!['my_assembly.ClassA'].docs!.custom!.exampleMetadata = + 'infused fixture=myfix.ts-fixture'; + await otherAssembly.updateAssembly(); + + // Expect to return cached snippet regardless of change + // No compilation should happen + const translationFunction = jest.fn().mockResolvedValue({ diagnostics: [], translatedSnippets: [] }); + await extract.extractSnippets([otherAssembly.moduleDirectory], { + cacheFromFile: cacheFile, + ...defaultExtractOptions, + translatorFactory: (o) => new MockTranslator(o, translationFunction), + }); + + expect(translationFunction).not.toHaveBeenCalled(); + } finally { + await otherAssembly.cleanup(); + } +}); + test('infused examples skip loose mode', async () => { const otherAssembly = await TestJsiiModule.fromSource( { @@ -464,7 +514,7 @@ test('infused examples skip loose mode', async () => { }), ).rejects.toThrowError(/Sample uses literate source/); - // Add infused=true to metadata and update assembly + // Add infused to metadata and update assembly otherAssembly.assembly.types!['my_assembly.ClassA'].docs!.custom!.exampleMetadata = 'lit=integ.test.ts infused'; await otherAssembly.updateAssembly(); From 349d9227ded1b386b5a38fb75d432c41c08e55cd Mon Sep 17 00:00:00 2001 From: kaizen3031593 Date: Thu, 23 Dec 2021 15:46:24 -0500 Subject: [PATCH 3/3] refactor infused tests --- .../test/commands/extract.test.ts | 107 ++++++++---------- 1 file changed, 47 insertions(+), 60 deletions(-) diff --git a/packages/jsii-rosetta/test/commands/extract.test.ts b/packages/jsii-rosetta/test/commands/extract.test.ts index 016b4eaaf6..2043cb7cf3 100644 --- a/packages/jsii-rosetta/test/commands/extract.test.ts +++ b/packages/jsii-rosetta/test/commands/extract.test.ts @@ -432,94 +432,83 @@ test('extract and infuse in one command', async () => { expect(types!['my_assembly.ClassA'].docs?.example).toBeDefined(); }); -test('infused examples always returned from cache', async () => { - const otherAssembly = await TestJsiiModule.fromSource( - { - 'index.ts': ` - /** - * ClassA - * - * @exampleMetadata infused - * @example x - */ - export class ClassA { - public someMethod() { +describe('infused examples', () => { + let infusedAssembly: TestJsiiModule; + beforeEach(async () => { + infusedAssembly = await TestJsiiModule.fromSource( + { + 'index.ts': ` + /** + * ClassA + * + * @exampleMetadata infused + * @example x + */ + export class ClassA { + public someMethod() { + } } - } - `, - }, - { - name: 'my_assembly', - jsii: DUMMY_JSII_CONFIG, - }, - ); - try { - const cacheFile = path.join(otherAssembly.moduleDirectory, 'test.tabl.json'); + `, + }, + { + name: 'my_assembly', + jsii: DUMMY_JSII_CONFIG, + }, + ); + }); + + afterEach(async () => { + await infusedAssembly.cleanup(); + }); + + test('always returned from cache', async () => { + const cacheFile = path.join(infusedAssembly.moduleDirectory, 'test.tabl.json'); // Cache to file - await extract.extractSnippets([otherAssembly.moduleDirectory], { + await extract.extractSnippets([infusedAssembly.moduleDirectory], { cacheToFile: cacheFile, ...defaultExtractOptions, }); // Update the example with a fixture that would fail compilation - otherAssembly.assembly.types!['my_assembly.ClassA'].docs!.custom!.exampleMetadata = + // Nothing like this should happen in practice + infusedAssembly.assembly.types!['my_assembly.ClassA'].docs!.custom!.exampleMetadata = 'infused fixture=myfix.ts-fixture'; - await otherAssembly.updateAssembly(); + await infusedAssembly.updateAssembly(); // Expect to return cached snippet regardless of change // No compilation should happen const translationFunction = jest.fn().mockResolvedValue({ diagnostics: [], translatedSnippets: [] }); - await extract.extractSnippets([otherAssembly.moduleDirectory], { + await extract.extractSnippets([infusedAssembly.moduleDirectory], { cacheFromFile: cacheFile, ...defaultExtractOptions, translatorFactory: (o) => new MockTranslator(o, translationFunction), }); expect(translationFunction).not.toHaveBeenCalled(); - } finally { - await otherAssembly.cleanup(); - } -}); + }); -test('infused examples skip loose mode', async () => { - const otherAssembly = await TestJsiiModule.fromSource( - { - 'index.ts': ` - /** - * ClassA - * - * @exampleMetadata lit=integ.test.ts - * @example x - */ - export class ClassA { - public someMethod() { - } - } - `, - }, - { - name: 'my_assembly', - jsii: DUMMY_JSII_CONFIG, - }, - ); - try { - const cacheToFile = path.join(otherAssembly.moduleDirectory, 'test.tabl.json'); + test('skip loose mode', async () => { + // Remove infused for now and add lit metadata that should fail + infusedAssembly.assembly.types!['my_assembly.ClassA'].docs!.custom!.exampleMetadata = 'lit=integ.test.ts'; + await infusedAssembly.updateAssembly(); + + const cacheToFile = path.join(infusedAssembly.moduleDirectory, 'test.tabl.json'); // Without exampleMetadata infused, expect an error await expect( - extract.extractSnippets([otherAssembly.moduleDirectory], { + extract.extractSnippets([infusedAssembly.moduleDirectory], { cacheToFile, ...defaultExtractOptions, }), ).rejects.toThrowError(/Sample uses literate source/); // Add infused to metadata and update assembly - otherAssembly.assembly.types!['my_assembly.ClassA'].docs!.custom!.exampleMetadata = 'lit=integ.test.ts infused'; - await otherAssembly.updateAssembly(); + infusedAssembly.assembly.types!['my_assembly.ClassA'].docs!.custom!.exampleMetadata = 'lit=integ.test.ts infused'; + await infusedAssembly.updateAssembly(); // Expect same function call to succeed now - await extract.extractSnippets([otherAssembly.moduleDirectory], { + await extract.extractSnippets([infusedAssembly.moduleDirectory], { cacheToFile, ...defaultExtractOptions, }); @@ -528,9 +517,7 @@ test('infused examples skip loose mode', async () => { expect(tablet.count).toEqual(1); const tr = tablet.tryGetSnippet(tablet.snippetKeys[0]); expect(tr?.originalSource.source).toEqual('x'); - } finally { - await otherAssembly.cleanup(); - } + }); }); class MockTranslator extends RosettaTranslator {