Feat network steps #173
Open
Feat network steps #173
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Nelsonochoam
reviewed
Dec 9, 2022
...testing-platform-api/src/main/java/com/aws/greengrass/testing/platform/PlatformResolver.java
Outdated
Show resolved
Hide resolved
Nelsonochoam
reviewed
Dec 9, 2022
aws-greengrass-testing-platform/aws-greengrass-testing-platform-api/pom.xml
Outdated
Show resolved
Hide resolved
aws-greengrass-testing-platform/aws-greengrass-testing-platform-api/pom.xml
Outdated
Show resolved
Hide resolved
jaiszeen
force-pushed
the
feat_network_steps
branch
from
eccbd3a
to
7ccc866
Compare
saranyailla
reviewed
Dec 12, 2022
...-features-api/src/main/java/com/aws/greengrass/testing/features/CommanConnectivitySteps.java
Outdated
Show resolved
Hide resolved
saranyailla
reviewed
Dec 12, 2022
| @@ -124,7 +124,7 @@ | |||
| <dependency> | |||
| <groupId>com.google.guava</groupId> | |||
| <artifactId>guava</artifactId> | |||
| <version>31.1-jre</version> | |||
| <version>23.0</version> | |||
There was a problem hiding this comment.
Mmm I don't know why this shows up as changed but we have to make sure this has the previous value. We don't want to update this dependencies if it is not needed.
saranyailla
reviewed
Dec 13, 2022
...-features-api/src/main/java/com/aws/greengrass/testing/features/CommanConnectivitySteps.java
Outdated
Show resolved
Hide resolved
| @@ -194,11 +194,6 @@ | |||
| <version>${auto.service.version}</version> | |||
| <scope>provided</scope> | |||
| </dependency> | |||
| <dependency> | |||
There was a problem hiding this comment.
Why was this dependency removed?
...-features-api/src/main/java/com/aws/greengrass/testing/features/CommonConnectivitySteps.java
Outdated
Show resolved
Hide resolved
...ting-platform-api/src/main/java/com/aws/greengrass/testing/platform/linux/LinuxPlatform.java
Outdated
Show resolved
Hide resolved
...-features-api/src/main/java/com/aws/greengrass/testing/features/CommanConnectivitySteps.java
Outdated
Show resolved
Hide resolved
...-features-api/src/main/java/com/aws/greengrass/testing/features/CommonConnectivitySteps.java
Outdated
Show resolved
Hide resolved
...-features-api/src/main/java/com/aws/greengrass/testing/features/CommanConnectivitySteps.java
Outdated
Show resolved
Hide resolved
...-features-api/src/main/java/com/aws/greengrass/testing/features/CommonConnectivitySteps.java
Outdated
Show resolved
Hide resolved
...-features-api/src/main/java/com/aws/greengrass/testing/features/CommanConnectivitySteps.java
Outdated
Show resolved
Hide resolved
...-features-api/src/main/java/com/aws/greengrass/testing/features/CommonConnectivitySteps.java
Outdated
Show resolved
Hide resolved
...-features-api/src/main/java/com/aws/greengrass/testing/features/CommonConnectivitySteps.java
Outdated
Show resolved
Hide resolved
...esting-features-api/src/main/java/com/aws/greengrass/testing/features/ConnectivitySteps.java
Outdated
Show resolved
Hide resolved
...esting-features-api/src/main/java/com/aws/greengrass/testing/features/ConnectivitySteps.java
Outdated
Show resolved
Hide resolved
...esting-features-api/src/main/java/com/aws/greengrass/testing/features/ConnectivitySteps.java
Outdated
Show resolved
Hide resolved
...esting-features-api/src/main/java/com/aws/greengrass/testing/features/ConnectivitySteps.java
Outdated
Show resolved
Hide resolved
...esting-features-api/src/main/java/com/aws/greengrass/testing/features/ConnectivitySteps.java
Outdated
Show resolved
Hide resolved
...esting-features-api/src/main/java/com/aws/greengrass/testing/features/ConnectivitySteps.java
Outdated
Show resolved
Hide resolved
...-platform-api/src/main/java/com/aws/greengrass/testing/platform/linux/NetworkUtilsLinux.java
Show resolved
Hide resolved
...esting-features-api/src/main/java/com/aws/greengrass/testing/features/ConnectivitySteps.java
Outdated
Show resolved
Hide resolved
| <artifactId>utils</artifactId> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>com.google.inject</groupId> |
There was a problem hiding this comment.
This dependency is not needed here
Nelsonochoam
previously approved these changes
Dec 13, 2022
saranyailla
reviewed
Dec 13, 2022
| */ | ||
| @When("the device network connectivity is {word}") | ||
| public void setDeviceNetwork(final String connectivity) throws IOException, InterruptedException { | ||
| switch (connectivity) { |
There was a problem hiding this comment.
the device network connectivity is OFFLINE is considered as a wrong input here. Do we want that?
There was a problem hiding this comment.
I believe so, there are only 2 branches
saranyailla
reviewed
Dec 13, 2022
| throw new UnsupportedOperationException("Connectivity " + connectivity + " is not supported"); | ||
| } | ||
|
|
||
| offline = connectivity.equalsIgnoreCase("offline"); |
There was a problem hiding this comment.
In general, this can throw null pointer exception if connectivity is null . Just reverse the check here - "offline".equalsIgnoreCase(connectivity). Also, this can be moved to L40 where you can directly update offline to true
There was a problem hiding this comment.
Great call out. I like the reversing of order to avoid the NPE. I would suggest not moving it to L40. There are scenarios where the client might go offline, then online and we want to flip the flag to avoid triggering the recover step on teardown
| throw new UnsupportedOperationException("Connectivity " + connectivity + " is not supported"); | ||
| } | ||
|
|
||
| "offline".equalsIgnoreCase(connectivity); |
...esting-features-api/src/main/java/com/aws/greengrass/testing/features/ConnectivitySteps.java
Outdated
Show resolved
Hide resolved
saranyailla
previously approved these changes
Dec 13, 2022
Nelsonochoam
approved these changes
Dec 13, 2022
saranyailla
approved these changes
Dec 13, 2022
|
UAT Test Passing: https://github.com/aws-greengrass/aws-greengrass-testing/actions/runs/3690242190/jobs/6247046788 on |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of changes:
Port over common connectivity steps from EF to OTF, which are used by several tests that are being ported into repositories using OTF as a dependency
Why is this change necessary:
Without it we can not simulate scenarios were the network connection is lost.
How was this change tested:
Built OTF locally and used the step on a test that is being ported for the log manager.