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.

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

Relationship e2e tests #15016

Merged
merged 1 commit into from
Sep 10, 2020
Merged

Conversation

abhipsaMisra
Copy link
Member

No description provided.

@ghost ghost added the DigitalTwins label Sep 10, 2020
@@ -75,6 +75,12 @@
<version>5.6.2</version> <!-- {x-version-update;org.junit.jupiter:junit-jupiter-params;external_dependency} -->
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.assertj</groupId>
Copy link
Member Author

Choose a reason for hiding this comment

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

Using assertj for fluent assertions, let me know what you guys think.

* @return A {@link PagedIterable} of application/json relationships directed towards the specified digital twin and the http response.
*/
@ServiceMethod(returns = ReturnType.COLLECTION)
public PagedIterable<IncomingRelationship> listIncomingRelationships(String digitalTwinId) {
Copy link
Member Author

Choose a reason for hiding this comment

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

This API overload was missing.

@abhipsaMisra abhipsaMisra force-pushed the relationshipTest branch 2 times, most recently from d61b725 to 335a9ca Compare September 10, 2020 20:01
.as("Created relationship from room -> floor");
logger.info("Created {} relationship between source = {} and target = {}", roomFloorRelationship.getId(), roomFloorRelationship.getSourceId(), roomFloorRelationship.getTargetId());

// Create a relation which already exists - should return status code 409 (Conflict).
Copy link
Member

Choose a reason for hiding this comment

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

Could this be split out into a separate test?

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought about that, but that would require that we create the model and twins again. Since they were already available as a part of this lifecycle test, I added this assertion in here directly.

client.deleteRelationship(hvacTwinId, HVAC_COOLS_FLOOR_RELATIONSHIP_ID);
logger.info("Deleted relationship {} for twin {}", HVAC_COOLS_FLOOR_RELATIONSHIP_ID, hvacTwinId);

// GET a relationship which doesn't exist - should return status code 404 (Not Found).
Copy link
Member

Choose a reason for hiding this comment

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

Could this be split out into a separate test? This test is already quite long

Copy link
Member Author

@abhipsaMisra abhipsaMisra Sep 10, 2020

Choose a reason for hiding this comment

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

I can split it into a separate test if you feel strongly about it; I added it in here because of the convenience (the models are twins were already created).

.verifyErrorSatisfies(ex -> assertRestException(ex, HTTP_PRECON_FAILED));

// Update relationships

Copy link
Member

Choose a reason for hiding this comment

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

Like in the sync client test, are you missing code here?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, the update operation is in the line right below this comment. Is the additional spacing confusing? Or is it because of the comment "Create relationship from Floor -> Room"?

import java.util.function.Function;

public class UniqueIdHelper {
public static String getUniqueModelId(String baseName, DigitalTwinsAsyncClient client, Function<Integer, String> randomIntegerStringGenerator) {
return getUniqueId(baseName, (modelId -> client.getModel(modelId).block().getId()), randomIntegerStringGenerator);
return getUniqueId(baseName, (modelId -> Objects.requireNonNull(client.getModel(modelId).block()).getId()), randomIntegerStringGenerator);
Copy link
Contributor

Choose a reason for hiding this comment

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

don't we need it everywhere in this file?

Copy link
Member Author

Choose a reason for hiding this comment

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

the required non-null check? I added it here because intellij recommended that I do; we should go through our code to see if it makes sense to add it elsewhere as well.

@@ -0,0 +1,276 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we just call it RelationshipsAsyncTest? do we need the DigitalTwin prefix?

Copy link
Member Author

Choose a reason for hiding this comment

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

I followed the naming pattern that we've used in our .NET samples

@abhipsaMisra abhipsaMisra merged commit ceb25b8 into Azure:master Sep 10, 2020
@abhipsaMisra abhipsaMisra deleted the relationshipTest branch September 10, 2020 20:40
openapi-sdkautomation bot pushed a commit to AzureSDKAutomation/azure-sdk-for-java that referenced this pull request Jul 2, 2021
Changes to fix the Resource Provider name casing issue for Azure Postgres, MySQL and MariaDB  (Azure#15016)

* Changes to fix the Resource provided name casing issue for Azure Postgres, MySQL and MariaDB services

* Changes to fix the Resource provided name casing issue for Azure Postgres, MySQL and MariaDB services

* Revert "Changes to fix the Resource provided name casing issue for Azure Postgres, MySQL and MariaDB services"

This reverts commit ecc38dc560b77de750350143fd80bdd6e3823882.
openapi-sdkautomation bot pushed a commit to AzureSDKAutomation/azure-sdk-for-java that referenced this pull request Jul 2, 2021
Changes to fix the Resource Provider name casing issue for Azure Postgres, MySQL and MariaDB  (Azure#15016)

* Changes to fix the Resource provided name casing issue for Azure Postgres, MySQL and MariaDB services

* Changes to fix the Resource provided name casing issue for Azure Postgres, MySQL and MariaDB services

* Revert "Changes to fix the Resource provided name casing issue for Azure Postgres, MySQL and MariaDB services"

This reverts commit ecc38dc560b77de750350143fd80bdd6e3823882.
openapi-sdkautomation bot pushed a commit to AzureSDKAutomation/azure-sdk-for-java that referenced this pull request Jul 2, 2021
Changes to fix the Resource Provider name casing issue for Azure Postgres, MySQL and MariaDB  (Azure#15016)

* Changes to fix the Resource provided name casing issue for Azure Postgres, MySQL and MariaDB services

* Changes to fix the Resource provided name casing issue for Azure Postgres, MySQL and MariaDB services

* Revert "Changes to fix the Resource provided name casing issue for Azure Postgres, MySQL and MariaDB services"

This reverts commit ecc38dc560b77de750350143fd80bdd6e3823882.
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.

3 participants