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.

Sign up for GitHub

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

Jump to bottom

Update Repo structure batch 4 #6362

Merged
merged 5 commits into from
May 28, 2019
Merged

Update Repo structure batch 4 #6362

merged 5 commits into from
May 28, 2019

Conversation

chidozieononiwu
Copy link
Member

  • Updated service location repo root/sdk
  • Updated package structure.
  • Renamed projects to start with Microsoft.Azure.Batch
  • Updated to project properties to match new engineering system.
  • Updated TargetFramework to use $(RequiredTargetFramework) i.e. net452, netstandard1.4, net461 and netcoreapp2.1 for test projects.
  • Updated CertificateBuilder.cs to run for netcore using BountyCastle instead of Mono.Security.
  • Updated CertificateUnitTests.cs
  • Disabled CerficaCredentials return on IntegrationTestCommon.cs
  • Used Async methods to accomodate .netcore2.1
  • Added [LiveTest] Attribute for test requiring environment variable.
  • Removed tests in IntegrationJobApplicationPackageReferencesTests.cs due to wrong parameters in BatchServiceClient

matthchr
matthchr requested changes May 22, 2019
View reviewed changes
Copy link
Member

@matthchr matthchr left a comment

Choose a reason for hiding this comment

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

Took a look, it looks mostly ok, had a few comments.

eng/.docsettings.yml Show resolved Hide resolved
sdk/batch/Microsoft.Azure.Batch/Microsoft.Azure.Batch.sln Outdated Show resolved Hide resolved
sdk/batch/Microsoft.Azure.Batch/src/Microsoft.Azure.Batch.csproj Outdated Show resolved Hide resolved
sdk/batch/Microsoft.Azure.Batch/src/Microsoft.Azure.Batch.csproj Outdated Show resolved Hide resolved
sdk/batch/Microsoft.Azure.Batch/tests/IntegrationTestCommon/CertificateBuilder.cs Show resolved Hide resolved
...Microsoft.Azure.Batch/tests/IntegrationTests/IntegrationTestUtilities/FileUploadUtilities.cs Outdated Show resolved Hide resolved
....Azure.Batch/tests/IntegrationTests/IntegrationTestUtilities/SynchronizationContextHelper.cs Outdated Show resolved Hide resolved
.../Microsoft.Azure.Batch/tests/IntegrationTests/Microsoft.Azure.Batch.Integration.Tests.csproj Show resolved Hide resolved
...Microsoft.Azure.Batch/tests/ProtocolTests/IntegrationJobApplicationPackageReferencesTests.cs Outdated Show resolved Hide resolved
...Microsoft.Azure.Batch/tests/ProtocolTests/IntegrationJobApplicationPackageReferencesTests.cs Outdated Show resolved Hide resolved
@chidozieononiwu
Copy link
Member Author

@Azure/azure-sdk-eng

@chidozieononiwu chidozieononiwu force-pushed the UpdateRepoStructure_Batch_4 branch from 67b9ee7 to ec45e06 Compare May 23, 2019 21:04
@chidozieononiwu chidozieononiwu marked this pull request as ready for review May 23, 2019 21:24
@chidozieononiwu chidozieononiwu force-pushed the UpdateRepoStructure_Batch_4 branch from ec45e06 to e2add16 Compare May 23, 2019 23:07
@chidozieononiwu chidozieononiwu requested a review from matthchr May 23, 2019 23:09
@chidozieononiwu chidozieononiwu force-pushed the UpdateRepoStructure_Batch_4 branch 2 times, most recently from 6660c60 to 5a34d14 Compare May 24, 2019 17:41
@weshaggard
Copy link
Member

/azp run net - client

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@chidozieononiwu
Copy link
Member Author

/azp run net - client

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@chidozieononiwu
Copy link
Member Author

/azp run net - batch - ci

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

@chidozieononiwu
Copy link
Member Author

/azp run net - batch - ci

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@chidozieononiwu
Copy link
Member Author

/azp run net - batch - ci

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@chidozieononiwu
Copy link
Member Author

/azp run net - batch - ci

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@chidozieononiwu
Copy link
Member Author

/azp run net - batch - ci

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@chidozieononiwu
Copy link
Member Author

/azp run net - batch - ci

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@chidozieononiwu chidozieononiwu force-pushed the UpdateRepoStructure_Batch_4 branch from 1e14e09 to 0584ef0 Compare May 24, 2019 21:24
@chidozieononiwu
Copy link
Member Author

/azp run net - batch - ci

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@chidozieononiwu
Copy link
Member Author

/azp run net - batch - ci

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@chidozieononiwu chidozieononiwu force-pushed the UpdateRepoStructure_Batch_4 branch from 0584ef0 to 5c5b07e Compare May 24, 2019 21:31
@chidozieononiwu
Copy link
Member Author

/azp run net - batch - ci

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@chidozieononiwu
Copy link
Member Author

/azp run net - batch - ci

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@weshaggard
Copy link
Member

weshaggard commented May 28, 2019 •
edited
Loading

Looks like there are some warnings about test results because some of the test projects don't actually contain tests:

https://dev.azure.com/azure-sdk/public/_build/results?buildId=38551&view=logs - PublishTestResults step

Windows:

No Result Found to Publish 'D:\a\1\s\sdk\batch\Microsoft.Azure.Batch\tests\IntegrationTestCommon\TestResults\VssAdministrator_fv-az639_2019-05-25_01_57_42_499.trx'.
No Result Found to Publish 'D:\a\1\s\sdk\batch\Microsoft.Azure.Batch\tests\IntegrationTestCommon\TestResults\VssAdministrator_fv-az639_2019-05-25_01_57_57_715.trx'.
No Result Found to Publish 'D:\a\1\s\sdk\batch\Microsoft.Azure.Batch\tests\ProtocolTests\TestResults\VssAdministrator_fv-az639_2019-05-25_01_58_10_982.trx'.
No Result Found to Publish 'D:\a\1\s\sdk\batch\Microsoft.Azure.Batch\tests\ProtocolTests\TestResults\VssAdministrator_fv-az639_2019-05-25_01_58_14_742.trx'.
No Result Found to Publish 'D:\a\1\s\sdk\batch\Microsoft.Azure.Batch\tests\TestCommon\TestResults\VssAdministrator_fv-az639_2019-05-25_01_58_10_550.trx'.
No Result Found to Publish 'D:\a\1\s\sdk\batch\Microsoft.Azure.Batch\tests\TestCommon\TestResults\VssAdministrator_fv-az639_2019-05-25_01_58_12_335.trx'.

The linux and Mac give this warning instead:

##[warning]Invalid results file. Make sure the result format of the file '/home/vsts/work/1/s/sdk/batch/Microsoft.Azure.Batch/tests/IntegrationTestCommon/TestResults/_fv-az644_2019-05-25_01_56_30_382.trx' matches 'VSTest' test results format.
##[warning]Invalid results file. Make sure the result format of the file '/home/vsts/work/1/s/sdk/batch/Microsoft.Azure.Batch/tests/ProtocolTests/TestResults/_fv-az644_2019-05-25_01_56_41_929.trx' matches 'VSTest' test results format.
##[warning]Invalid results file. Make sure the result format of the file '/home/vsts/work/1/s/sdk/batch/Microsoft.Azure.Batch/tests/TestCommon/TestResults/_fv-az644_2019-05-25_01_56_33_026.trx' matches 'VSTest' test results format.

However I think they stem from the same issue which is there are not test results for those projects. We should either rename the projects or set IsTestProject=false (similar to https://github.com/Azure/azure-sdk-for-net/blob/master/eng/Directory.Build.Data.targets#L31) in the project to get them not to run tests and report no results.

@chidozieononiwu chidozieononiwu force-pushed the UpdateRepoStructure_Batch_4 branch from 8f79112 to c0ed880 Compare May 28, 2019 16:53
@chidozieononiwu
Copy link
Member Author

/azp run net - batch - ci

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@chidozieononiwu
Copy link
Member Author

Looks like there are some warnings about test results because some of the test projects don't actually contain tests:

https://dev.azure.com/azure-sdk/public/_build/results?buildId=38551&view=logs - PublishTestResults step

Windows:

No Result Found to Publish 'D:\a\1\s\sdk\batch\Microsoft.Azure.Batch\tests\IntegrationTestCommon\TestResults\VssAdministrator_fv-az639_2019-05-25_01_57_42_499.trx'.
No Result Found to Publish 'D:\a\1\s\sdk\batch\Microsoft.Azure.Batch\tests\IntegrationTestCommon\TestResults\VssAdministrator_fv-az639_2019-05-25_01_57_57_715.trx'.
No Result Found to Publish 'D:\a\1\s\sdk\batch\Microsoft.Azure.Batch\tests\ProtocolTests\TestResults\VssAdministrator_fv-az639_2019-05-25_01_58_10_982.trx'.
No Result Found to Publish 'D:\a\1\s\sdk\batch\Microsoft.Azure.Batch\tests\ProtocolTests\TestResults\VssAdministrator_fv-az639_2019-05-25_01_58_14_742.trx'.
No Result Found to Publish 'D:\a\1\s\sdk\batch\Microsoft.Azure.Batch\tests\TestCommon\TestResults\VssAdministrator_fv-az639_2019-05-25_01_58_10_550.trx'.
No Result Found to Publish 'D:\a\1\s\sdk\batch\Microsoft.Azure.Batch\tests\TestCommon\TestResults\VssAdministrator_fv-az639_2019-05-25_01_58_12_335.trx'.

The linux and Mac give this warning instead:

##[warning]Invalid results file. Make sure the result format of the file '/home/vsts/work/1/s/sdk/batch/Microsoft.Azure.Batch/tests/IntegrationTestCommon/TestResults/_fv-az644_2019-05-25_01_56_30_382.trx' matches 'VSTest' test results format.
##[warning]Invalid results file. Make sure the result format of the file '/home/vsts/work/1/s/sdk/batch/Microsoft.Azure.Batch/tests/ProtocolTests/TestResults/_fv-az644_2019-05-25_01_56_41_929.trx' matches 'VSTest' test results format.
##[warning]Invalid results file. Make sure the result format of the file '/home/vsts/work/1/s/sdk/batch/Microsoft.Azure.Batch/tests/TestCommon/TestResults/_fv-az644_2019-05-25_01_56_33_026.trx' matches 'VSTest' test results format.

However I think they stem from the same issue which is there are not test results for those projects. We should either rename the projects or set IsTestProject=false (similar to https://github.com/Azure/azure-sdk-for-net/blob/master/eng/Directory.Build.Data.targets#L31) in the project to get them not to run tests and report no results.

I have fixed the be renaming the common tests projects. I also deleted the protocol test as @matthchr requested. I guess this is ready to merge now.

matthchr
matthchr approved these changes May 28, 2019
View reviewed changes
Copy link
Member

@matthchr matthchr left a comment

Choose a reason for hiding this comment

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

Looks good now

@chidozieononiwu chidozieononiwu merged commit a42b1b6 into Azure:master May 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@weshaggard weshaggard weshaggard approved these changes

@matthchr matthchr matthchr approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@chidozieononiwu @weshaggard @matthchr