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

Add tests of client code generation components #13444

Merged
merged 4 commits into from
Aug 27, 2019

Conversation

dougbu
Copy link
Member

@dougbu dougbu commented Aug 26, 2019

Description

Test-only change to add tests of Microsoft.Extensions.ApiDescription.Client package

Customer impact

Increased confidence we're shipping something that works

Regression

no

Risk

Minimal. The tests are designed to work reliably and cross-platform.

- #4914
- include tests of tasks
- include test project that builds and can be tested in the future
@dougbu
Copy link
Member Author

dougbu commented Aug 26, 2019

@Pilchie are test-only changes in 'release/3.0' ask or tell mode?

@@ -0,0 +1,183 @@
{
Copy link
Member Author

Choose a reason for hiding this comment

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

The JSON files in this directory aren't particularly interesting. Suggest just marking them as Viewed.

@@ -23,22 +23,18 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.dotnet-openapi",
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "dotnet-microsoft.openapi.Tests", "Microsoft.dotnet-openapi\test\dotnet-microsoft.openapi.Tests.csproj", "{26BBA8A7-0F69-4C5F-B1C2-16B3320FFE3F}"
EndProject
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Extensions.ApiDescription.Client", "Extensions.ApiDescription.Client", "{78610083-1FCE-47F5-AB4D-AF0E1313B351}"
Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure when the hierarchy got changed but I moved the service ref projects to match.

@Pilchie
Copy link
Member

Pilchie commented Aug 26, 2019

Tell mode is fine for test only changes.

@dougbu dougbu added tell-mode Indicates a PR which is being merged during tell-mode Test labels Aug 26, 2019
@Pilchie Pilchie added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Aug 26, 2019
- add `MockBuildEngine` class to support tests where MSBuild task logs
- correct an incredibly minor issue related to logging errors for `@(OpenApiProjectReference)` items
  - set `%(SourceProject)` metadata and use it when logging

nit: use `SortedDictionary<string, string>` everwhere
@@ -48,7 +48,9 @@
</MSBuild>

<ItemGroup>
<OpenApiReference Include="@(_Temporary)" Exclude="@(OpenApiReference)" RemoveMetadata="FullConfiguration" />
<OpenApiReference Include="@(_Temporary)" Exclude="@(OpenApiReference)" RemoveMetadata="FullConfiguration">
<SourceProject>%(_Temporary.OriginalItemSpec)</SourceProject>
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like a change to the source. Does this need ask mode approval?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's teensy but I defer to @Pilchie w.r.t ask mode.

As mentioned in my description, this only affects logging of an error related to @(OpenApiProjectReference) items. %(OriginalItemSpec) metadata would be a viable alternative in GetOpenApiReferenceMetadata but %(SourceProject) is nice to have later because %(OriginalItemSpec) will exist in all items that task produces.

Copy link
Member

Choose a reason for hiding this comment

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

I'm okay with this.

@dougbu
Copy link
Member Author

dougbu commented Aug 26, 2019

hit again by a marked-flaky test running in main build ☹️

@dougbu dougbu merged commit 8ea3eda into release/3.0 Aug 27, 2019
@ghost ghost deleted the dougbu/service.ref.tests.4914 branch August 27, 2019 03:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates tell-mode Indicates a PR which is being merged during tell-mode
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants