Skip to content

Update tests for Policy SDK version 2018-03-01#4499

Merged
dsgouda merged 5 commits intoAzure:psSdkJson6from
mentat9:psSdkJson6
Jul 6, 2018
Merged

Update tests for Policy SDK version 2018-03-01#4499
dsgouda merged 5 commits intoAzure:psSdkJson6from
mentat9:psSdkJson6

Conversation

@mentat9
Copy link
Member

@mentat9 mentat9 commented Jun 27, 2018

Description

NOTE: This update does NOT include a swagger change. Consequently, the below boxes for updating generate.cmd/generate.ps1 and project versions are not checked.

This update adds tests to cover the most recent update to the Governance Policy swagger.

Current REST spec for Governance Policy is here:
https://github.com/Azure/azure-rest-api-specs/tree/master/specification/resources/resource-manager/Microsoft.Authorization/stable/2018-03-01
Most recent changes were in this PR:
Azure/azure-rest-api-specs#3032


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

General Guidelines

  • 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

  • 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.
  • 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.
  • The *.csproj and AssemblyInfo.cs files have been updated with the new version of the SDK.

Remove duplicate type created when ManagementGroup API was removed from the rest of the resources SDK
Refactor and greatly improve policy live/scenario tests
Add many new policy live tests
Save session records from recorded tests
@dsgouda
Copy link
Contributor

dsgouda commented Jun 28, 2018

@mentat9 Please fix the failing managementgroup tests. You may have to re-record the tests

Copy link
Contributor

@dsgouda dsgouda left a comment

Choose a reason for hiding this comment

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

Please regenerate the code using the generate.ps1 here
Please commit all the files generated/modified

@mentat9
Copy link
Member Author

mentat9 commented Jun 28, 2018

Please regenerate the code using the generate.ps1 here

Is it normal to do a regeneration when there is no swagger change?

@dsgouda
Copy link
Contributor

dsgouda commented Jun 28, 2018

@mentat9 I assumed you were adding changes to the SDK too. Please disregard the regeneration

@mentat9
Copy link
Member Author

mentat9 commented Jun 29, 2018

@mentat9 Please fix the failing managementgroup tests. You may have to re-record the tests

Thing is, those aren't my tests and they don't record successfully on my machine. Playback, OTOH is working fine, so I don't understand why they aren't working on the server. Any ideas?

@mentat9
Copy link
Member Author

mentat9 commented Jul 3, 2018

@dsgouda, the tests have been re-recorded, but they still fail in the server build. Neither I nor Raj (the test owner) know how to fix this break. These tests all run fine locally in both of our dev environments. Can you help?

@dsgouda
Copy link
Contributor

dsgouda commented Jul 3, 2018

@mentat9 taking a look

@mentat9
Copy link
Member Author

mentat9 commented Jul 5, 2018

@dsgouda did you learn anything about the break?

@dsgouda
Copy link
Contributor

dsgouda commented Jul 5, 2018

Tried re-running the build without success, will have to debug myself. @shahabhijeet could you take a look at it too?

@dsgouda
Copy link
Contributor

dsgouda commented Jul 5, 2018

@mentat9 Please check if you copied over the recorded test into the SessionRecords directory.
You can use this script too.

@shahabhijeet
Copy link
Contributor

@rajshah11 can you review this PR.

Copy link
Contributor

@rajshah11 rajshah11 left a comment

Choose a reason for hiding this comment

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

@dsgouda @shahabhijeet LGTM. I also ran the tests again and the changes made to ManagementGroups.Test project were fine. The build ran fine locally. Not sure why the build is failing here though.

@mentat9
Copy link
Member Author

mentat9 commented Jul 6, 2018

I think I might have figured it out. It looks like someone left behind some old unused test recordings under:

... \SDKs\Resource\Resource.Tests\SessionRecords\ResourceGroups.Tests.LiveManagementGroupsTests

When the management group SDK was removed from the resources SDK, these should have been deleted. I'm guessing server test discovery is finding these and trying to run them, but they are outdated.

Hopefully, that is the problem. If so, I don't understand why my build was the first to break. I would have expected every build since the MG code was removed from the Resource SDK should have broken.

@mentat9
Copy link
Member Author

mentat9 commented Jul 6, 2018

I have no idea what this failure in the Travis build means:

"The command "sudo apt-key adv --keyserver hkp://keyserver.ubuntu.com:80 --recv-keys 417A0893" failed and exited with 2 during ."

But I've seen it before and it had nothing to do with my PR: it was something broken in the build environment. @dsgouda (or someone), can you force a retry on the Travis build?

Copy link
Contributor

@dsgouda dsgouda left a comment

Choose a reason for hiding this comment

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

LGTM

@dsgouda dsgouda merged commit bb37279 into Azure:psSdkJson6 Jul 6, 2018
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.

4 participants