-
Notifications
You must be signed in to change notification settings - Fork 77
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
SLVS-1818 Upgrade SLCore to 10.14.0.80189 and adapt RPC changes for SQC multi-region #5985
Conversation
a52e452
to
2b0afa6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You missed adding the region for ListUserOrganizationsParams. Now we have an exception when trying to create a SQC connection.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some feedback regarding CaYC
@@ -176,7 +177,7 @@ public async Task GetOrganizationsAsync_TokenIsProvided_CallsSlCoreListUserOrgan | |||
|
|||
await testSubject.GetOrganizationsAsync(new TokenCredentialsModel(token.CreateSecureString())); | |||
|
|||
await connectionConfigurationSlCoreService.Received(1).ListUserOrganizationsAsync(Arg.Is<ListUserOrganizationsParams>(x=> IsExpectedCredentials(x.credentials, token))); | |||
await connectionConfigurationSlCoreService.Received(1).ListUserOrganizationsAsync(Arg.Is<ListUserOrganizationsParams>(x=> IsExpectedCredentials(x.credentials, token) && x.region == SonarCloudRegion.EU)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a helper method to check the region, i.e. IsExpectedRegion
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't really see the point of creating a method to replace one equals expression
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because this occurs in multiple places. And if something changes, it would be easier to just modify one helper method instead of multiple places.
@@ -192,10 +193,10 @@ public async Task GetOrganizationsAsync_CredentialsIsNull_ReturnsFailedResponseA | |||
[TestMethod] | |||
public async Task GetOrganizationsAsync_NoOrganizationExists_ReturnsSuccessResponseAndEmptyOrganizations() | |||
{ | |||
connectionConfigurationSlCoreService.ListUserOrganizationsAsync(Arg.Any<ListUserOrganizationsParams>()) | |||
connectionConfigurationSlCoreService.ListUserOrganizationsAsync(Arg.Is<ListUserOrganizationsParams>(x => x.credentials.Left.token == token && x.region == SonarCloudRegion.EU)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
connectionConfigurationSlCoreService.ListUserOrganizationsAsync(Arg.Is<ListUserOrganizationsParams>(x => x.credentials.Left.token == token && x.region == SonarCloudRegion.EU)) | |
connectionConfigurationSlCoreService.ListUserOrganizationsAsync(Arg.Is<ListUserOrganizationsParams>(x => IsExpectedCredentials(x.credentials, token) && IsExpectedRegion(x.region))) |
@@ -205,10 +206,11 @@ public async Task GetOrganizationsAsync_NoOrganizationExists_ReturnsSuccessRespo | |||
public async Task GetOrganizationsAsync_OrganizationExists_ReturnsSuccessResponseAndMappedOrganizations() | |||
{ | |||
List<OrganizationDto> serverOrganizations = [new OrganizationDto("key", "name", "desc"), new OrganizationDto("key2", "name2", "desc2")]; | |||
connectionConfigurationSlCoreService.ListUserOrganizationsAsync(Arg.Any<ListUserOrganizationsParams>()) | |||
|
|||
connectionConfigurationSlCoreService.ListUserOrganizationsAsync(Arg.Is<ListUserOrganizationsParams>(x => x.credentials.Left.token == token && x.region == SonarCloudRegion.EU)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
connectionConfigurationSlCoreService.ListUserOrganizationsAsync(Arg.Is<ListUserOrganizationsParams>(x => x.credentials.Left.token == token && x.region == SonarCloudRegion.EU)) | |
connectionConfigurationSlCoreService.ListUserOrganizationsAsync(Arg.Is<ListUserOrganizationsParams>(x => IsExpectedCredentials(x.credentials, token) && IsExpectedRegion(x.region))) |
@georgii-borovinskikh-sonarsource, this PR should target master. |
Quality Gate passedIssues Measures |
SLVS-1818