Skip to content

Commit

Permalink
fix(rosetta): rosetta extract fails if run after rosetta infuse (#…
Browse files Browse the repository at this point in the history
…3248)

There is a valid use case to run `rosetta extract` after `rosetta infuse`.
However, we run into problems because we `fixturize` snippets
before we check the cache, and sometimes `fixturizing` will fail for
infused snippets (specifically, literate sources like `integ.blah.lit.ts`
will fail).

We do not actually need to successfully `fixturize` infused examples.
We rely on these examples to be in the cached tablet file and thus
do not need to be compiled and translated again. So we introduce
a new metadata tag, `infuse=true` for all infused examples. This tag, 
when consumed by `extract`, will skip any errors associated with
that snippet. 

ALSO: increased test timeout from `30000` to `50000` to allow enough
time for mac PR test to succeed.

---

By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license].

[Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0
  • Loading branch information
kaizencc authored Dec 13, 2021
1 parent 710869b commit e3ec929
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 10 deletions.
15 changes: 9 additions & 6 deletions packages/jsii-rosetta/lib/commands/infuse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { renderMetadataline, TypeScriptSnippet } from '../snippet';
import { SnippetSelector, mean, meanLength, shortest, longest } from '../snippet-selectors';
import { snippetKey } from '../tablets/key';
import { LanguageTablet, TranslatedSnippet, DEFAULT_TABLET_NAME } from '../tablets/tablets';
import { isDefined, mkDict, fmap, indexBy } from '../util';
import { isDefined, mkDict, indexBy } from '../util';

export interface InfuseResult {
readonly coverageResults: Record<string, InfuseTypes>;
Expand Down Expand Up @@ -207,17 +207,20 @@ function insertExample(
type: spec.Type,
tablets: LanguageTablet[],
): void {
const exampleMetadata = fmap(original?.parameters, renderMetadataline);
const parameters = {
...original?.parameters,
infused: '',
};
// exampleMetadata should always be nonempty since we always have a parameter.
const exampleMetadata = renderMetadataline(parameters) ?? '';

if (type.docs) {
type.docs.example = example.originalSource.source;
if (exampleMetadata) {
type.docs.custom = { ...type.docs.custom, exampleMetadata };
}
type.docs.custom = { ...type.docs.custom, exampleMetadata };
} else {
type.docs = {
example: example.originalSource.source,
custom: fmap(exampleMetadata, (exampleMetadata) => ({ exampleMetadata })),
custom: { exampleMetadata },
};
}

Expand Down
8 changes: 6 additions & 2 deletions packages/jsii-rosetta/lib/jsii/assemblies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -159,12 +159,16 @@ export function allTypeScriptSnippets(assemblies: readonly LoadedAssembly[], loo
for (const source of allSnippetSources(assembly)) {
switch (source.type) {
case 'example':
// If an example is an infused example, we do not care about compiler errors.
// We are relying on the tablet cache to have this example stored already.
const [strictForExample, looseForExample] =
source.metadata?.infused !== undefined ? [false, true] : [strict, loose];
const location = { api: source.location, field: { field: 'example' } } as const;
const snippet = updateParameters(typeScriptSnippetFromSource(source.source, location, strict), {
const snippet = updateParameters(typeScriptSnippetFromSource(source.source, location, strictForExample), {
[SnippetParameters.$PROJECT_DIRECTORY]: directory,
...source.metadata,
});
ret.push(fixturize(snippet, loose));
ret.push(fixturize(snippet, looseForExample));
break;
case 'markdown':
for (const snippet of extractTypescriptSnippetsFromMarkdown(source.markdown, source.location, strict)) {
Expand Down
52 changes: 52 additions & 0 deletions packages/jsii-rosetta/test/commands/extract.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,58 @@ test('extract and infuse in one command', async () => {
expect(types!['my_assembly.ClassA'].docs?.example).toBeDefined();
});

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');

// Without exampleMetadata infused=true, expect an error
await expect(
extract.extractSnippets([otherAssembly.moduleDirectory], {
cacheToFile,
...defaultExtractOptions,
}),
).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';
await otherAssembly.updateAssembly();

// Expect same function call to succeed now
await extract.extractSnippets([otherAssembly.moduleDirectory], {
cacheToFile,
...defaultExtractOptions,
});

const tablet = await LanguageTablet.fromFile(cacheToFile);
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 {
public constructor(opts: RosettaTranslatorOptions, translatorFn: jest.Mock) {
super(opts);
Expand Down
2 changes: 1 addition & 1 deletion packages/jsii-rosetta/test/commands/infuse.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ test('infuse copies example metadata', async () => {
const updatedAssembly = (await fs.readJson(path.join(assembly.moduleDirectory, '.jsii'))) as spec.Assembly;

const typeDocs = updatedAssembly.types?.['my_assembly.ClassA']?.docs;
expect(typeDocs?.custom?.exampleMetadata).toEqual('some=metadata');
expect(typeDocs?.custom?.exampleMetadata).toEqual('some=metadata infused');
});

test('examples are added to the tablet under new keys', async () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/jsii/test/deprecation-warnings.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@ function testpkg_Baz(p) {
// Recompiling without deprecation warning to leave the packages in a clean state
await compile(calcBaseRoot, false);
await compile(calcLibRoot, false);
}, 30000);
}, 50000);
});

describe('Call injections', () => {
Expand Down

0 comments on commit e3ec929

Please sign in to comment.