Skip to content

Management.Sql - Generated SQL DataSync and added test scenario's#3558

Merged
shahabhijeet merged 5 commits intoAzure:psSdkJson6from
ScottHolden:psSdkJson6
Aug 14, 2017
Merged

Management.Sql - Generated SQL DataSync and added test scenario's#3558
shahabhijeet merged 5 commits intoAzure:psSdkJson6from
ScottHolden:psSdkJson6

Conversation

@ScottHolden
Copy link
Copy Markdown
Contributor

Description

Generated Management.Sql with package-2015-05-preview api spec including datasync, added SyncAgent SyncGroup SyncMember scenario's with session records.

Corresponding rest-api-spec change can be found at:
Azure/azure-rest-api-specs#1510


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.

…ing datasync, added SyncAgent SyncGroup SyncMember scenario's with session records.
@azuresdkci
Copy link
Copy Markdown
Contributor

Can one of the admins verify this patch?

2 similar comments
@azuresdkci
Copy link
Copy Markdown
Contributor

Can one of the admins verify this patch?

@azuresdkci
Copy link
Copy Markdown
Contributor

Can one of the admins verify this patch?

@msftclas
Copy link
Copy Markdown

msftclas commented Aug 8, 2017

@ScottHolden,
Thanks for your contribution.
To ensure that the project team has proper rights to use your work, please complete the Contribution License Agreement at https://cla.microsoft.com.

It will cover your contributions to all Microsoft-managed open source projects.
Thanks,
Microsoft Pull Request Bot

@azuresdkci
Copy link
Copy Markdown
Contributor

Can one of the admins verify this patch?

@msftclas
Copy link
Copy Markdown

msftclas commented Aug 8, 2017

@ScottHolden, thanks for signing the contribution license agreement. We will now validate the agreement and then the pull request.

Thanks, Microsoft Pull Request Bot

@ScottHolden
Copy link
Copy Markdown
Contributor Author

I have signed the cla :)

@shahabhijeet
Copy link
Copy Markdown
Contributor

@ScottHolden are you part of the Azure org, if not please become a part of the Azure Team by visiting
https://github.com/Azure/

@jaredmoo
Copy link
Copy Markdown
Contributor

jaredmoo commented Aug 8, 2017

@fhljys ,@ferno-ms , @nathannfan fyi

Copy link
Copy Markdown
Contributor

@jaredmoo jaredmoo left a comment

Choose a reason for hiding this comment

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

Looks excellent, just some minor changes requested. Thank you!

<Description>Azure SQL Management SDK library</Description>
<AssemblyName>Microsoft.Azure.Management.Sql</AssemblyName>
<VersionPrefix>1.7.0-preview</VersionPrefix>
<VersionPrefix>1.8.0-preview</VersionPrefix>
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 latest published version is 1.6.0 (see https://www.nuget.org/packages/Microsoft.Azure.Management.Sql ) so please undo this

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done :)




[assembly: AssemblyFileVersion("1.8.0.0")] No newline at end of file
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.

please undo

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done :)

});
Assert.NotNull(updateSyncMember);
Assert.Equal(updateSyncMemberDirection, updateSyncMember.SyncDirection);
Assert.NotEqual(syncMemberDirection, updateSyncMemberDirection);
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 also test empty update, e.g.

// Empty update of sync member
SyncMember updateSyncMember2 = sqlClient.SyncMembers.Update(resourceGroup.Name, server.Name, testDatabaseName, syncGroupName, syncMemberName, new SyncMember());

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done :)

IPage<SyncMember> listSyncMembers = sqlClient.SyncMembers.ListBySyncGroup(resourceGroup.Name, server.Name, testDatabaseName, syncGroupName);
Assert.NotNull(listSyncMembers);
Assert.Equal(1, listSyncMembers.Count());
Assert.Equal(syncMemberName, listSyncMembers.FirstOrDefault()?.Name);
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.

nit: already checked that there is one, so this can be listSyncMembers.Single().Name

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done :)

Interval = interval2
});
Assert.NotNull(updateSyncGroup);
Assert.Equal(interval2, updateSyncGroup.Interval);
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 please test empty update, i..e.

SyncGroup updateSyncGroup = sqlClient.SyncGroups.Update(resourceGroup.Name, server.Name, testDatabaseName, syncGroupName, new SyncGroup());

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done :)

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.

oops, these empty updates ideally should not fail. Not your fault though, it has to be fixed server-side. Thanks for testing!

@ScottHolden
Copy link
Copy Markdown
Contributor Author

@shahabhijeet I'm not currently part of the Azure GitHub organisation, who do I need to reach out to to be added, as I couldn't see a link on https://github.com/Azure/

@jaredmoo
Copy link
Copy Markdown
Contributor

jaredmoo commented Aug 9, 2017

@ScottHolden I think that this is only applicable to Microsoft employees working in the Azure organization. So my understanding is that this is not applicable to you.

@shahabhijeet
Copy link
Copy Markdown
Contributor

@azuresdkci test this please

@shahabhijeet
Copy link
Copy Markdown
Contributor

@ScottHolden sql tests are failing on travis.
Were you able to successfully run SQL tests locally?
Click on Details and check the failures.

@shahabhijeet
Copy link
Copy Markdown
Contributor

@azuresdkci add to whitelist

@ScottHolden
Copy link
Copy Markdown
Contributor Author

All tests were working fine locally @shahabhijeet. I was able to run in record mode and playback mode, and committed the SessionRecord with the test changes.
I'll have a look into what is happening and try and replicate locally :)

@ScottHolden
Copy link
Copy Markdown
Contributor Author

ScottHolden commented Aug 14, 2017

Running locally gives me no errors at all, I can't seem to replicate :(

image

image

@jaredmoo
Copy link
Copy Markdown
Contributor

It looks like the lab build merges with the target branch (azure/psSdkJson6) before running tests, and we made some incompatible test changes recently that you haven't pulled in. It should be fixed by merging from azure/psSdkJson6 and re-recording the tests. You will need to add a few new properties to your test environment connection string, e.g. DefaultLocationId=japaneast;DefaultLocation=Japan East.

@ScottHolden
Copy link
Copy Markdown
Contributor Author

Thanks @jaredmoo ! That did the trick :)
Should be fine to merge now :)

Copy link
Copy Markdown
Contributor

@shahabhijeet shahabhijeet left a comment

Choose a reason for hiding this comment

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

@jaredmoo I assume you will be updating the AssemblyInfo to reflect the next release version.

@shahabhijeet shahabhijeet merged commit 5b5bbd8 into Azure:psSdkJson6 Aug 14, 2017
@jaredmoo
Copy link
Copy Markdown
Contributor

Awesome! Yes AssemblyInfo is handled. Thx

JasonYang-MSFT pushed a commit to JasonYang-MSFT/azure-sdk-for-net that referenced this pull request Dec 8, 2017
…ure#3558)

* Generated Management.Sql with package-2015-05-preview api spec including datasync, added SyncAgent SyncGroup SyncMember scenario's with session records.

* Reverted version to 1.7.0, made minor changes to scenarios, re-recorded scenarios

* Regenerated sql management client now rest api specs have been merged.

* Re-recorded DataSync test scenarios
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.

5 participants