Skip to content

Introduce mock test base#22445

Closed
ArcturusZhang wants to merge 4 commits intoAzure:feature/mgmt-track2from
ArcturusZhang:dapzhang/mock-test-base
Closed

Introduce mock test base#22445
ArcturusZhang wants to merge 4 commits intoAzure:feature/mgmt-track2from
ArcturusZhang:dapzhang/mock-test-base

Conversation

@ArcturusZhang
Copy link
Copy Markdown
Member

@ArcturusZhang ArcturusZhang commented Jul 5, 2021

Prerequisite PR: #22524

All SDK Contribution checklist:

This checklist is used to make sure that common guidelines for a pull request are followed.

  • Please open PR in Draft mode if it is:
    • Work in progress or not intended to be merged.
    • Encountering multiple pipeline failures and working on fixes.
  • If an SDK is being regenerated based on a new swagger spec, a link to the pull request containing these swagger spec changes has been included above.
  • I have read the contribution guidelines.
  • The pull request does not introduce breaking changes.

General Guidelines and Best Practices

  • Title of the pull request is clear and informative.
  • There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, see this page.

Testing Guidelines

  • Pull request includes test coverage for the included changes.

SDK Generation Guidelines

  • The generate.cmd file for the SDK has been updated with the version of AutoRest, as well as the commitid of your swagger spec or link to the swagger spec, used to generate the code. (Track 2 only)
  • The *.csproj and AssemblyInfo.cs files have been updated with the new version of the SDK. Please double check nuget.org current release version.

Additional management plane SDK specific contribution checklist:

Note: Only applies to Microsoft.Azure.Management.[RP] or Azure.ResourceManager.[RP]

  • Include updated management metadata.
  • Update AzureRP.props to add/remove version info to maintain up to date API versions.

Management plane SDK Troubleshooting

  • If this is very first SDK for a services and you are adding new service folders directly under /SDK, please add new service label and/or contact assigned reviewer.

  • If the check fails at the Verify Code Generation step, please ensure:

    • Do not modify any code in generated folders.
    • Do not selectively include/remove generated files in the PR.
    • Do use generate.ps1/cmd to generate this PR instead of calling autorest directly.
      Please pay attention to the @microsoft.csharp version output after running generate.ps1. If it is lower than current released version (2.3.82), please run it again as it should pull down the latest version.

    Note: We have recently updated the PSH module called by generate.ps1 to emit additional data. This would help reduce/eliminate the Code Verification check error. Please run following command:

      `dotnet msbuild eng/mgmt.proj /t:Util /p:UtilityName=InstallPsModules`
    

Old outstanding PR cleanup

Please note:
If PRs (including draft) has been out for more than 60 days and there are no responses from our query or followups, they will be closed to maintain a concise list for our reviewers.

@ghost ghost added the Azure.Core label Jul 5, 2021
@ArcturusZhang ArcturusZhang force-pushed the dapzhang/mock-test-base branch 2 times, most recently from 3305bee to 74eaf1f Compare July 5, 2021 08:14
@ArcturusZhang ArcturusZhang marked this pull request as ready for review July 5, 2021 09:11
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The only purpose of this base class seems to be to diable instrumentation validation. Not worth having a class for that.

Copy link
Copy Markdown
Member Author

@ArcturusZhang ArcturusZhang Jul 8, 2021

Choose a reason for hiding this comment

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

And it turns out this overriding is also not needed.

I believe it is very possible that we could only use the MockTestEnvironment to indicate the test was a mock test.

@m-nash what do you think?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You don't need a test environment for mock tests because they would never run live.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

That is true, but I need the Credential to be the mock credential even not in Playback mode

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you use new MockCredential()?

@ArcturusZhang ArcturusZhang force-pushed the dapzhang/mock-test-base branch 7 times, most recently from 1f2d7fb to 3f87b34 Compare July 9, 2021 05:32
@ArcturusZhang ArcturusZhang force-pushed the dapzhang/mock-test-base branch from cac8a1c to 314fcae Compare July 13, 2021 06:39
@m-nash m-nash closed this Jul 14, 2021
@m-nash m-nash deleted the branch Azure:feature/mgmt-track2 July 14, 2021 20:49
@ArcturusZhang ArcturusZhang deleted the dapzhang/mock-test-base branch July 15, 2021 06:28
@ArcturusZhang ArcturusZhang mentioned this pull request Jul 15, 2021
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants