Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor the sample generation into rlc-common & Generate samples with mock content #2002

Merged
merged 94 commits into from
Oct 17, 2023

Conversation

MaryGao
Copy link
Member

@MaryGao MaryGao commented Sep 4, 2023

fixes #2000

The pr is mainly focusing on generating samples if no real examples provided. Here is the high-level design:

rlc-common

  • transformSampleGroups.ts

In common layer we provide a transform function which could generate sample content based on path/schema/operation details in RLCModel. Currently we only provide mock value generation which could be improved if we have suggested values in future.

export function transformSampleGroups(model: RLCModel, allowMockValue = true)
  • valueGenerationUtil.ts

Helper functions to help generate values, the idea here is to try to suggest the ts-type from schema, if can't we will fallback to suggest ts-type from the generated type iteself, e.g Record<string, "test" | 1> would be a record type.

export function generateParameterTypeValue(
  type: string,
  parameterName: string,
  schemaMap: Map<string, Schema>,
  path: Set<string> = new Set(),
  _allowMockValue = true
)
  • typeUtil.ts

Helper utils to detect the detailed ts types.

typespec-ts

The only thing in typespec project needs to do is apply the common transform logic and small improvements in schema for better detection.

model.sampleGroups = transformSampleGroups(
    model,
    options?.generateSample ===
      true /* Enable mock sample content if generateSample === true */
  );

autorest.typescript

  • rlcSampleGenerator.ts

We prefer to generate sample from swagger examples first and if no swagger examples, we will generate mock sample.

Testing

  • rlc-common/test/helpers/typeUtil.spec.ts testing for different type convertion
  • typespec-ts/test/unit/transform/transformSchemas.spec.ts testing for distinct schema data
  • typespec-ts/test/unit/sample/generateSampleContent.spec.ts integration testing from typespec to generated values
  • smoke testing both in swagger and typespec

@MaryGao MaryGao changed the title Refactor the sample generation into rlc-common Refactor the sample generation into rlc-common & Generate samples with fake content Sep 11, 2023
@MaryGao MaryGao changed the title Refactor the sample generation into rlc-common & Generate samples with fake content Refactor the sample generation into rlc-common & Generate samples with mock content Sep 11, 2023
@qiaozha
Copy link
Member

qiaozha commented Oct 16, 2023

leave some comments

Copy link
Member

@qiaozha qiaozha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just some minor comments

sample.methodParamAssignments = getAssignmentStrArray(parameters.method);
sample.methodParamNames = parameters.method.length > 0 ? "options" : "";
sample.methodParamNames =
parameters.method.length > 0 ? parameters.method[0].value ?? "" : "";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a little curious about the priority between ? and ??. should we use parentheses to be explicit here ?

Copy link
Member Author

@MaryGao MaryGao Oct 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The priority should be good if i added parentheses explicitly the formatter would remove them automatically.

parameters.method.length > 0 ? (parameters.method[0].value ?? "") : "";

I mean something like true[] or 12[] or "test"[]

updated.

Copy link
Member

@qiaozha qiaozha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just some minor comments

@MaryGao MaryGao merged commit a1b5026 into Azure:main Oct 17, 2023
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Modular] Generate samples based fake content
3 participants