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

Samples as tests #2215

Merged

Conversation

timotheeguerin
Copy link
Member

@timotheeguerin timotheeguerin commented Jul 27, 2023

fix #3017

Run the samples as test which allows all of them to run without crashing on the first one, lets us run it in the test explorer.

This also make the sample regeneration much faster(typespec azure repo):

  • Before ~90s
  • Now ~12s

image

New resolveCompilerOptions utils

This also include the addition of a new util resolveCompilerOptions that resolve the compiler options programtically in the same way that the cli would have for a given entrypoint.

I can move this into a dedicated PR if people prefer.

@github-actions
Copy link
Contributor

Changes in this PR will be published to the following url to try(check status of TypeSpec Pull Request Try It pipeline for publish status):
Playground: https://cadlplayground.z22.web.core.windows.net/prs/2215/

Website: https://cadlwebsite.z1.web.core.windows.net/prs/2215/

Copy link
Member Author

@timotheeguerin timotheeguerin Jul 28, 2023

Choose a reason for hiding this comment

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

Rename of the files is due to it including the emitter name in the output path (Default output-dir)

So it goes from

-/test/output/authentication/openapi.yaml
+/test/output/authentication/@typespec/openapi3/openapi.yaml

This is useful if we want to run multiple emitters for the same sample which is something we do in the typespec-azure repo where is a lot of rename as well https://github.com/Azure/typespec-azure/pull/3337

@timotheeguerin timotheeguerin marked this pull request as ready for review July 28, 2023 14:30
Copy link
Member Author

Choose a reason for hiding this comment

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

I can move this new util into a dedicated PR if people prefer for the review.

Copy link
Contributor

@markcowl markcowl left a comment

Choose a reason for hiding this comment

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

This looks great, a couple of comments and questions

packages/samples/package.json Show resolved Hide resolved
packages/samples/src/sample-snapshot-testing.ts Outdated Show resolved Hide resolved
@timotheeguerin timotheeguerin merged commit 31fd5ab into microsoft:main Aug 4, 2023
@timotheeguerin timotheeguerin deleted the feature/samples-snapshot-tests branch August 4, 2023 17:19
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.

2 participants