[GitHub] [trafficcontrol] Daisykh commented on a change in pull request #3573: Client steering forced diversity

[GitBox <gi...@apache.org>]

Daisykh commented on a change in pull request #3573: Client steering forced diversity
URL: https://github.com/apache/trafficcontrol/pull/3573#discussion_r284351660
 
 

 ##########
 File path: traffic_router/core/src/test/java/com/comcast/cdn/traffic_control/traffic_router/core/external/SteeringTest.java
 ##########
 @@ -553,4 +556,54 @@ public void itUsesMultiLocationFormatWithMoreThanTwoEntries() throws Exception {
 			if (response != null) { response.close(); }
 		}
 	}
+
+	@Test
+	public void itSupportsClientSteeringDiversity() throws Exception {
+		final String path = "/foo?fakeClientIpAddress=12.34.56.78";
+		HttpGet httpGet = new HttpGet("http://localhost:" + routerHttpPort + path);
+		httpGet.addHeader("Host", "cdn.client-steering-diversity-test.thecdn.example.com");
+
+		CloseableHttpResponse response = null;
+
+		try {
+			response = httpClient.execute(httpGet);
+
+			HttpEntity entity = response.getEntity();
+			assertThat("Failed getting 302 for request " + httpGet.getFirstHeader("Host").getValue(), response.getStatusLine().getStatusCode(), equalTo(302));
+
+			ObjectMapper objectMapper = new ObjectMapper(new JsonFactory());
+
+			assertThat(entity.getContent(), not(nullValue()));
+
+			JsonNode json = objectMapper.readTree(entity.getContent());
+
+			assertThat(json.has("locations"), equalTo(true));
+			assertThat(json.get("locations").size(), equalTo(5));
+
+			Set<String> actualEdges = new HashSet<>();
+			Set<String> actualTargets = new HashSet<>();
+
+			for (JsonNode n : json.get("locations")) {
+				String l = n.asText();
+				String[] parts = l.split("\\.");
+				parts[0] = parts[0].replaceFirst("http://", "");
+				actualEdges.add(parts[0]);
+				actualTargets.add(parts[1]);
+			}
+
+			// with client.steering.forced.diversity = true, expect to get all 3 edges back in the locations
+			// by default (client.steering.forced.diversity = false), only one edge would be returned in the locations
+
+			String[] expectedEdgesArray = {"edge-cache-csd-1", "edge-cache-csd-2", "edge-cache-csd-3"};
 
 Review comment:
   Would it be beneficial to add some more testing on some edge cases? Here's what my initial thoughts were:
   - Test on no locations (empty failure case)
   - Test on 2 locations available only (I think this one is pretty important)
   - Test on different DS weighting values

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services