-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Live tests #2812
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
Live tests #2812
Changes from 3 commits
d9901ca
083fef5
6154137
d204813
e767308
f2655db
585e5c2
c3c4e88
e226db2
2682bd8
efc825b
e221646
9022fb9
9faef73
805ed55
fdaa2f2
0232b36
e22b3e5
addfe30
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| resources: | ||
| - repo: self | ||
| queue: | ||
| name: Hosted VS2017 | ||
| steps: | ||
| - bash: | | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a reason you aren't using the Maven@3 task to run the tests similar to https://github.com/Azure/azure-sdk-for-java/blob/master/.azure-pipelines/client.test.yml#L17? In general we should try to do that to help be consistent and also that allows us to capture the test results into the test tab of the build which gives us a good view of the tests than ran and a history of them.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The environment variables were not read during the build when tried to configure as Maven task. |
||
| LOG_PARAMS='-Dorg.slf4j.simpleLogger.defaultLogLevel=error -Dorg.slf4j.simpleLogger.log.org.apache.maven.cli.transfer.Slf4jMavenTransferListener=warn' | ||
| mvn test -Dsurefire.rerunFailingTestsCount=3 $LOG_PARAMS -Dparallel=classes -DthreadCount=3 -DforkCount=1C -f pom.client.build.xml | ||
| displayName: 'Bash Script' | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please update this to represent what it is doing, like "Running Live Tests"
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated it. |
||
| env: | ||
| ARM_CLIENTID: $(ARM_CLIENTID) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are these 2 env variables the only difference between live vs not live?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are more Variables defined in the variables tab of dev ops pipeline. |
||
| ARM_CLIENTKEY: $(ARM_CLIENTKEY) | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,32 +2,20 @@ trigger: | |
| - master | ||
|
|
||
| jobs: | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like you have some merge conflicts to handle which should get this file updated to what is currently committed in the repo. As it stands you are reverting some changes that were committed because your branch is behind master.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Conflicts resolved |
||
| - template: client.test.yml | ||
| parameters: | ||
| name: Linux | ||
| vmImage: 'ubuntu-16.04' | ||
|
|
||
| - template: client.test.yml | ||
| parameters: | ||
| name: macOS | ||
| vmImage: 'macOS-10.13' | ||
|
|
||
| - template: client.test.yml | ||
| parameters: | ||
| name: Windows | ||
| vmImage: 'vs2017-win2016' | ||
|
|
||
| - job: 'Publish' | ||
|
|
||
| dependsOn: | ||
| - 'Test_Linux' | ||
| - 'Test_macOS' | ||
| - 'Test_Windows' | ||
|
|
||
| pool: | ||
| vmImage: 'ubuntu-16.04' | ||
|
|
||
| steps: | ||
| - task: Maven@3 | ||
| displayName: 'Checkstyle' | ||
| inputs: | ||
| mavenPomFile: 'pom.client.build.xml' | ||
| goals: 'checkstyle:check' | ||
| options: '--batch-mode' | ||
| mavenOptions: '-Dorg.slf4j.simpleLogger.defaultLogLevel=error -Dorg.slf4j.simpleLogger.log.org.apache.maven.cli.transfer.Slf4jMavenTransferListener=warn' | ||
| publishJUnitResults: false | ||
| - task: Maven@3 | ||
| inputs: | ||
| mavenPomFile: 'pom.client.build.xml' | ||
|
|
@@ -40,6 +28,14 @@ jobs: | |
| goals: 'package' | ||
| displayName: 'Package' | ||
|
|
||
| - task: Maven@3 | ||
| displayName: 'Javadoc' | ||
| inputs: | ||
| mavenPomFile: 'pom.client.build.xml' | ||
| goals: 'javadoc:jar' | ||
| options: '-DskipTests=true' | ||
| publishJUnitResults: false | ||
|
|
||
| - task: CopyFiles@2 | ||
| inputs: | ||
| contents: '**/*.jar' | ||
|
|
@@ -49,3 +45,18 @@ jobs: | |
|
|
||
| - task: PublishBuildArtifacts@1 | ||
| displayName: 'Publish' | ||
|
|
||
| - template: client.test.yml | ||
| parameters: | ||
| name: Linux | ||
| vmImage: 'ubuntu-16.04' | ||
|
|
||
| - template: client.test.yml | ||
| parameters: | ||
| name: macOS | ||
| vmImage: 'macOS-10.13' | ||
|
|
||
| - template: client.test.yml | ||
| parameters: | ||
| name: Windows | ||
| vmImage: 'vs2017-win2016' | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,38 +1,38 @@ | ||
| <?xml version="1.0"?> | ||
| <!DOCTYPE Configure PUBLIC "-//Jetty//Configure//EN" "http://www.eclipse.org/jetty/configure_9_0.dtd"> | ||
| <Configure id="Server" class="org.eclipse.jetty.server.Server"> | ||
| <Call name="addConnector"> | ||
| <Arg> | ||
| <New class="org.eclipse.jetty.server.ServerConnector"> | ||
| <Arg name="server"> | ||
| <Ref refid="Server" /> | ||
| </Arg> | ||
| <Arg name="factories"> | ||
| <Array type="org.eclipse.jetty.server.ConnectionFactory"> | ||
| <Item> | ||
| <New class="org.eclipse.jetty.server.HttpConnectionFactory"/> | ||
| </Item> | ||
| </Array> | ||
| </Arg> | ||
| <Set name="port">11080</Set> | ||
| </New> | ||
| </Arg> | ||
| </Call> | ||
| <Call name="addConnector"> | ||
| <Arg> | ||
| <New class="org.eclipse.jetty.server.ServerConnector"> | ||
| <Arg name="server"> | ||
| <Ref refid="Server" /> | ||
| </Arg> | ||
| <Arg name="factories"> | ||
| <Array type="org.eclipse.jetty.server.ConnectionFactory"> | ||
| <Item> | ||
| <New class="org.eclipse.jetty.server.HttpConnectionFactory"/> | ||
| </Item> | ||
| </Array> | ||
| </Arg> | ||
| <Set name="port">11081</Set> | ||
| </New> | ||
| </Arg> | ||
| </Call> | ||
| <?xml version="1.0"?> | ||
| <!DOCTYPE Configure PUBLIC "-//Jetty//Configure//EN" "http://www.eclipse.org/jetty/configure_9_0.dtd"> | ||
| <Configure id="Server" class="org.eclipse.jetty.server.Server"> | ||
| <Call name="addConnector"> | ||
| <Arg> | ||
| <New class="org.eclipse.jetty.server.ServerConnector"> | ||
| <Arg name="server"> | ||
| <Ref refid="Server" /> | ||
| </Arg> | ||
| <Arg name="factories"> | ||
| <Array type="org.eclipse.jetty.server.ConnectionFactory"> | ||
| <Item> | ||
| <New class="org.eclipse.jetty.server.HttpConnectionFactory"/> | ||
| </Item> | ||
| </Array> | ||
| </Arg> | ||
| <Set name="port">11080</Set> | ||
| </New> | ||
| </Arg> | ||
| </Call> | ||
| <Call name="addConnector"> | ||
| <Arg> | ||
| <New class="org.eclipse.jetty.server.ServerConnector"> | ||
| <Arg name="server"> | ||
| <Ref refid="Server" /> | ||
| </Arg> | ||
| <Arg name="factories"> | ||
| <Array type="org.eclipse.jetty.server.ConnectionFactory"> | ||
| <Item> | ||
| <New class="org.eclipse.jetty.server.HttpConnectionFactory"/> | ||
| </Item> | ||
| </Array> | ||
| </Arg> | ||
| <Set name="port">11081</Set> | ||
| </New> | ||
| </Arg> | ||
| </Call> | ||
| </Configure> |
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,7 +4,6 @@ | |
| import com.microsoft.aad.adal4j.AuthenticationResult; | ||
| import com.microsoft.aad.adal4j.ClientCredential; | ||
| import com.microsoft.azure.AzureResponseBuilder; | ||
| import com.microsoft.azure.credentials.ApplicationTokenCredentials; | ||
| import com.microsoft.azure.keyvault.KeyVaultClient; | ||
| import com.microsoft.azure.keyvault.authentication.KeyVaultCredentials; | ||
| import com.microsoft.azure.keyvault.models.Attributes; | ||
|
|
@@ -78,11 +77,11 @@ public static String getSecondaryVaultUri() { | |
| } | ||
|
|
||
| private static String getLiveVaultUri1() { | ||
| return getenvOrDefault("keyvault.vaulturi", "https://javasdktestvault.vault.azure.net"); | ||
| return getenvOrDefault("KEYVAULT_VAULTURI", "https://javasdktestvault.vault.azure.net"); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Out of curiosity - what motivated the changing of all key names to uppercase with underscores rather that periods?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like periods are not processed correctly on linux system, and VSTS converts them to underscore, and converts environment variable names to uppercase anyway. Please see https://developercommunity.visualstudio.com/content/problem/328064/vsts-silently-converts-variable-names-to-uppercase.html for uppercase and https://stackoverflow.com/questions/50020314/escaping-environment-variable-names-in-vsts for underscores. |
||
| } | ||
|
|
||
| private static String getLiveVaultUri2() { | ||
| return getenvOrDefault("keyvault.vaulturi.alt", "https://javasdktestvault2.vault.azure.net"); | ||
| return getenvOrDefault("KEYVAULT_VAULTURI_ALT", "https://javasdktestvault2.vault.azure.net"); | ||
| } | ||
|
|
||
| private static String getenvOrDefault(String varName, String defValue) { | ||
|
|
@@ -100,13 +99,13 @@ protected static void compareAttributes(Attributes expectedAttributes, Attribute | |
|
|
||
| private static AuthenticationResult getAccessToken(String authorization, String resource) throws Exception { | ||
|
|
||
| String clientId = System.getenv("arm.clientid"); | ||
| String clientId = System.getenv("ARM_CLIENTID"); | ||
|
|
||
| if (clientId == null) { | ||
| throw new Exception("Please inform arm.clientid in the environment settings."); | ||
| } | ||
|
|
||
| String clientKey = System.getenv("arm.clientkey"); | ||
| String clientKey = System.getenv("ARM_CLIENTKEY"); | ||
| String username = System.getenv("arm.username"); | ||
| String password = System.getenv("arm.password"); | ||
|
|
||
|
|
@@ -351,7 +350,7 @@ protected static DeletedKeyBundle pollOnKeyDeletion(String vaultBaseUrl, String | |
|
|
||
| protected static DeletedSecretBundle pollOnSecretDeletion(String vaultBaseUrl, String secretName) throws Exception { | ||
| int pendingPollCount = 0; | ||
| while (pendingPollCount < 50) { | ||
| while (pendingPollCount < 70) { | ||
| DeletedSecretBundle deletedSecretBundle = keyVaultClient.getDeletedSecret(vaultBaseUrl, secretName); | ||
| if (deletedSecretBundle == null) { | ||
| if (isRecordMode()) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -117,7 +117,7 @@ public void KeyVault_KeyVaultKeyResolver_Secret128Base64_KeyVaultKeyResolverDefa | |
| byte[] CEK = { 0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, (byte) 0x88, (byte) 0x99, (byte) 0xAA, (byte) 0xBB, (byte) 0xCC, (byte) 0xDD, (byte) 0xEE, (byte) 0xFF }; | ||
| byte[] EK = { 0x1F, (byte) 0xA6, (byte) 0x8B, 0x0A, (byte) 0x81, 0x12, (byte) 0xB4, 0x47, (byte) 0xAE, (byte) 0xF3, 0x4B, (byte) 0xD8, (byte) 0xFB, 0x5A, 0x7B, (byte) 0x82, (byte) 0x9D, 0x3E, (byte) 0x86, 0x23, 0x71, (byte) 0xD2, (byte) 0xCF, (byte) 0xE5 }; | ||
|
|
||
| String TEST_SECRET_NAME = SECRET_NAME + "1"; | ||
| String TEST_SECRET_NAME = SECRET_NAME + "2"; | ||
|
|
||
| try { | ||
| SetSecretRequest request = new SetSecretRequest.Builder(getVaultUri(), TEST_SECRET_NAME, _base64.encodeAsString(keyBytes)).withContentType("application/octet-stream").build(); | ||
|
|
@@ -181,7 +181,7 @@ public void KeyVault_KeyVaultKeyResolver_Secret192Base64_KeyVaultKeyResolverDefa | |
| byte[] CEK = { 0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, (byte) 0x88, (byte) 0x99, (byte) 0xAA, (byte) 0xBB, (byte) 0xCC, (byte) 0xDD, (byte) 0xEE, (byte) 0xFF }; | ||
| byte[] EK = { (byte) 0x96, 0x77, (byte) 0x8B, 0x25, (byte) 0xAE, 0x6C, (byte) 0xA4, 0x35, (byte) 0xF9, 0x2B, 0x5B, (byte) 0x97, (byte) 0xC0, 0x50, (byte) 0xAE, (byte) 0xD2, 0x46, (byte) 0x8A, (byte) 0xB8, (byte) 0xA1, 0x7A, (byte) 0xD8, 0x4E, 0x5D }; | ||
|
|
||
| String TEST_SECRET_NAME = SECRET_NAME + "2"; | ||
| String TEST_SECRET_NAME = SECRET_NAME + "4"; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It always makes me a little nervous when I see magic numbers being changed in unit tests. Why is this required?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tests involve creating and deleting secrets. Multiple Tests using same magic number for the secret name conflicts when a test is trying to create a secret and the previous Test requested a deletion. And the deletion is still in queue. |
||
|
|
||
| try { | ||
| SetSecretRequest request = new SetSecretRequest.Builder(getVaultUri(), TEST_SECRET_NAME, _base64.encodeAsString(keyBytes)).withContentType("application/octet-stream").build(); | ||
|
|
||
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.
Why do you need "resources: self"?
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.
Not needed actually,
removed.