Redis: Added system test #6375
Redis: Added system test #6375
Conversation
|
@chingor13 Build failed due to Redis service is not enable or activate for this project. |
There was a problem hiding this comment.
We likely want to create the instance in a @BeforeClass and delete it in the @AfterClass teardown. We also need to make sure this can run multiple times in parallel by avoiding using the same instance name/id.
...e-cloud-clients/google-cloud-redis/src/test/java/com/google/cloud/redis/it/ITSystemTest.java
Outdated
Show resolved
Hide resolved
|
@athakor I enabled the Redis API on the test project and restarted the test. It's now failing with a validation error about an invalid network parameter. |
Codecov Report
@@ Coverage Diff @@
## master #6375 +/- ##
============================================
- Coverage 46.5% 46.34% -0.16%
+ Complexity 28131 27968 -163
============================================
Files 2625 2613 -12
Lines 289532 287931 -1601
Branches 33753 33756 +3
============================================
- Hits 134637 133452 -1185
+ Misses 144654 144259 -395
+ Partials 10241 10220 -21
Continue to review full report at Codecov.
|
| /** Creates a Redis instance based on the specified tier and memory size. */ | ||
| LocationName parent = LocationName.of(projectId, LOCATION); | ||
| Instance instance = Instance.newBuilder().setTier(TIER).setMemorySizeGb(MEMORY_SIZE_GB).build(); | ||
| client.createInstanceAsync(parent, INSTANCE, instance).get(); |
There was a problem hiding this comment.
This is failing with an invalid config: Invalid authorized network 'default'. Legacy networks are not supported.
There was a problem hiding this comment.
@chingor13 Its working for grass-clump-project locally, from console as well as api explorer. Does it need any setting on project thats used for CI? Otherwise we will have to create VPC run system test and delete VPC which would add dependency on compute client.
There was a problem hiding this comment.
This is likely because our test project is very old and has some older grandfathered in settings (like default networking) still configured. I'll investigate some more.
There was a problem hiding this comment.
I'm hesitant to change the default network for the test project, so I created a new redis-vpc network in our test project. Let's make the test configurable (via a maven param like -Dredis.network=redis-vpc which we can add here (and in the continuous integration config). We can default that value to default for most other users who run the tests.
|
@chingor13 PTAL |
There was a problem hiding this comment.
Thanks, one last suggestion.
| INSTANCE_NAME_PREFIX + "-" + UUID.randomUUID().toString().substring(0, 8); | ||
| private static final String LOCATION = "us-central1"; | ||
| private static final int MEMORY_SIZE_GB = 1; | ||
| private static final String AUTHORIZED_NETWORK = System.getProperty("redis.network"); |
There was a problem hiding this comment.
| private static final String AUTHORIZED_NETWORK = System.getProperty("redis.network"); | |
| private static final String AUTHORIZED_NETWORK = System.getProperty("redis.network", "default"); |
| String authorizedNetwork = | ||
| AUTHORIZED_NETWORK == null || AUTHORIZED_NETWORK.isEmpty() | ||
| ? "projects/" + projectId + "/global/networks/default" | ||
| : "projects/" + projectId + "/global/networks/" + AUTHORIZED_NETWORK; |
There was a problem hiding this comment.
We can simplify this if we set the default above:
String authorizedNetwork = "projects/" + projectId + "/global/networks/" + AUTHORIZED_NETWORK;
towards #5947