[GitHub] flink pull request #5665: [FLINK-8703][tests] Port WebFrontendITCase to Mini...

2018-04-19 Thread asfgit 
Github user asfgit closed the pull request at:

https://github.com/apache/flink/pull/5665


---

[GitHub] flink pull request #5665: [FLINK-8703][tests] Port WebFrontendITCase to Mini...

2018-04-16 Thread aljoscha 
Github user aljoscha commented on a diff in the pull request:

https://github.com/apache/flink/pull/5665#discussion_r181777925
  
--- Diff: 
flink-runtime-web/src/test/java/org/apache/flink/runtime/webmonitor/WebFrontendITCase.java
 ---
@@ -217,12 +231,12 @@ public void getTaskManagerLogAndStdoutFiles() {
@Test
public void getConfiguration() {
try {
-   String config = 
TestBaseUtils.getFromHTTP("http://localhost:"; + port + "/jobmanager/config");
+   String config = 
TestBaseUtils.getFromHTTP("http://localhost:"; + CLUSTER.getWebUIPort() + 
"/jobmanager/config");
 
Map conf = 
WebMonitorUtils.fromKeyValueJsonArray(config);
assertEquals(
-   
cluster.configuration().getString("taskmanager.numberOfTaskSlots", null),
-   
conf.get(ConfigConstants.TASK_MANAGER_NUM_TASK_SLOTS));
+   
CLUSTER_CONFIGURATION.getString(ConfigConstants.LOCAL_START_WEBSERVER, null),
--- End diff --

Yeah, that's what I meant, the previous code was a bit strange


---

[GitHub] flink pull request #5665: [FLINK-8703][tests] Port WebFrontendITCase to Mini...

2018-04-16 Thread zentol 
Github user zentol commented on a diff in the pull request:

https://github.com/apache/flink/pull/5665#discussion_r181776571
  
--- Diff: 
flink-runtime-web/src/test/java/org/apache/flink/runtime/webmonitor/WebFrontendITCase.java
 ---
@@ -217,12 +231,12 @@ public void getTaskManagerLogAndStdoutFiles() {
@Test
public void getConfiguration() {
try {
-   String config = 
TestBaseUtils.getFromHTTP("http://localhost:"; + port + "/jobmanager/config");
+   String config = 
TestBaseUtils.getFromHTTP("http://localhost:"; + CLUSTER.getWebUIPort() + 
"/jobmanager/config");
 
Map conf = 
WebMonitorUtils.fromKeyValueJsonArray(config);
assertEquals(
-   
cluster.configuration().getString("taskmanager.numberOfTaskSlots", null),
-   
conf.get(ConfigConstants.TASK_MANAGER_NUM_TASK_SLOTS));
+   
CLUSTER_CONFIGURATION.getString(ConfigConstants.LOCAL_START_WEBSERVER, null),
--- End diff --

I wanted an option for which the configured value is different from the 
default. The default for `numberOfTaskSlots` is one, so the test would've 
passed even if the handler returned a vanilla configuration.


---

[GitHub] flink pull request #5665: [FLINK-8703][tests] Port WebFrontendITCase to Mini...

2018-04-16 Thread zentol 
Github user zentol commented on a diff in the pull request:

https://github.com/apache/flink/pull/5665#discussion_r181775179
  
--- Diff: 
flink-runtime-web/src/test/java/org/apache/flink/runtime/webmonitor/WebFrontendITCase.java
 ---
@@ -127,14 +137,18 @@ public void testResponseHeaders() throws Exception {
Assert.assertEquals("application/json; charset=UTF-8", 
taskManagerConnection.getContentType());
 
// check headers in case of an error
-   URL notFoundJobUrl = new URL("http://localhost:"; + port + 
"/jobs/dontexist");
+   URL notFoundJobUrl = new URL("http://localhost:"; + 
CLUSTER.getWebUIPort() + "/jobs/dontexist");
HttpURLConnection notFoundJobConnection = (HttpURLConnection) 
notFoundJobUrl.openConnection();
notFoundJobConnection.setConnectTimeout(10);
notFoundJobConnection.connect();
if (notFoundJobConnection.getResponseCode() >= 400) {
// we don't set the content-encoding header

Assert.assertNull(notFoundJobConnection.getContentEncoding());
-   Assert.assertEquals("text/plain; charset=UTF-8", 
notFoundJobConnection.getContentType());
+   if (Objects.equals("flip6", 
System.getProperty("codebase"))) {
--- End diff --

this must be updated to "new"


---

[GitHub] flink pull request #5665: [FLINK-8703][tests] Port WebFrontendITCase to Mini...

2018-04-16 Thread aljoscha 
Github user aljoscha commented on a diff in the pull request:

https://github.com/apache/flink/pull/5665#discussion_r181756207
  
--- Diff: 
flink-runtime-web/src/test/java/org/apache/flink/runtime/webmonitor/testutils/HttpTestClient.java
 ---
@@ -185,6 +185,25 @@ public void sendDeleteRequest(String path, 
FiniteDuration timeout) throws Timeou
sendRequest(getRequest, timeout);
}
 
+   /**
+* Sends a simple PATH request to the given path. You only specify the 
$path part of
+* http://$host:$host/$path.
+*
+* @param path The $path to DELETE (http://$host:$host/$path)
--- End diff --

copy error? `DELETE`


---

[GitHub] flink pull request #5665: [FLINK-8703][tests] Port WebFrontendITCase to Mini...

2018-04-16 Thread aljoscha 
Github user aljoscha commented on a diff in the pull request:

https://github.com/apache/flink/pull/5665#discussion_r181756063
  
--- Diff: 
flink-runtime-web/src/test/java/org/apache/flink/runtime/webmonitor/testutils/HttpTestClient.java
 ---
@@ -185,6 +185,25 @@ public void sendDeleteRequest(String path, 
FiniteDuration timeout) throws Timeou
sendRequest(getRequest, timeout);
}
 
+   /**
+* Sends a simple PATH request to the given path. You only specify the 
$path part of
--- End diff --

typo: PATH -> PATCH?


---

[GitHub] flink pull request #5665: [FLINK-8703][tests] Port WebFrontendITCase to Mini...

2018-04-16 Thread aljoscha 
Github user aljoscha commented on a diff in the pull request:

https://github.com/apache/flink/pull/5665#discussion_r181755757
  
--- Diff: 
flink-runtime-web/src/test/java/org/apache/flink/runtime/webmonitor/WebFrontendITCase.java
 ---
@@ -242,32 +256,44 @@ public void testStop() throws Exception {
final JobGraph jobGraph = new JobGraph("Stoppable streaming 
test job", sender);
final JobID jid = jobGraph.getJobID();
 
-   cluster.submitJobDetached(jobGraph);
+   ClusterClient clusterClient = CLUSTER.getClusterClient();
+   clusterClient.setDetached(true);
+   clusterClient.submitJob(jobGraph, 
WebFrontendITCase.class.getClassLoader());
 
// wait for job to show up
-   while (cluster.getCurrentlyRunningJobsJava().isEmpty()) {
+   while (getRunningJobs(CLUSTER.getClusterClient()).isEmpty()) {
Thread.sleep(10);
}
 
final FiniteDuration testTimeout = new FiniteDuration(2, 
TimeUnit.MINUTES);
final Deadline deadline = testTimeout.fromNow();
 
-   while (!cluster.getCurrentlyRunningJobsJava().isEmpty()) {
-   try (HttpTestClient client = new 
HttpTestClient("localhost", port)) {
-   // Request the file from the web server
-   client.sendDeleteRequest("/jobs/" + jid + 
"/stop", deadline.timeLeft());
-   HttpTestClient.SimpleHttpResponse response = 
client.getNextResponse(deadline.timeLeft());
-
-   assertEquals(HttpResponseStatus.OK, 
response.getStatus());
-   assertEquals("application/json; charset=UTF-8", 
response.getType());
-   assertEquals("{}", response.getContent());
+   while (!getRunningJobs(CLUSTER.getClusterClient()).isEmpty()) {
+   try (HttpTestClient client = new 
HttpTestClient("localhost", CLUSTER.getWebUIPort())) {
+   if 
(Objects.equals(MiniClusterResource.FLIP6_CODEBASE, 
System.getProperty(MiniClusterResource.CODEBASE_KEY))) {
+   // Request the file from the web server
--- End diff --

This comment is/was outdated, it seems.


---

[GitHub] flink pull request #5665: [FLINK-8703][tests] Port WebFrontendITCase to Mini...

2018-04-16 Thread aljoscha 
Github user aljoscha commented on a diff in the pull request:

https://github.com/apache/flink/pull/5665#discussion_r181755304
  
--- Diff: 
flink-runtime-web/src/test/java/org/apache/flink/runtime/webmonitor/WebFrontendITCase.java
 ---
@@ -217,12 +231,12 @@ public void getTaskManagerLogAndStdoutFiles() {
@Test
public void getConfiguration() {
try {
-   String config = 
TestBaseUtils.getFromHTTP("http://localhost:"; + port + "/jobmanager/config");
+   String config = 
TestBaseUtils.getFromHTTP("http://localhost:"; + CLUSTER.getWebUIPort() + 
"/jobmanager/config");
 
Map conf = 
WebMonitorUtils.fromKeyValueJsonArray(config);
assertEquals(
-   
cluster.configuration().getString("taskmanager.numberOfTaskSlots", null),
-   
conf.get(ConfigConstants.TASK_MANAGER_NUM_TASK_SLOTS));
+   
CLUSTER_CONFIGURATION.getString(ConfigConstants.LOCAL_START_WEBSERVER, null),
--- End diff --

what was up here? 😅 


---

[GitHub] flink pull request #5665: [FLINK-8703][tests] Port WebFrontendITCase to Mini...

2018-03-12 Thread zentol 
Github user zentol commented on a diff in the pull request:

https://github.com/apache/flink/pull/5665#discussion_r173743909
  
--- Diff: 
flink-runtime-web/src/test/java/org/apache/flink/runtime/webmonitor/WebFrontendITCase.java
 ---
@@ -66,40 +73,44 @@
private static final int NUM_TASK_MANAGERS = 2;
private static final int NUM_SLOTS = 4;
 
-   private static LocalFlinkMiniCluster cluster;
+   private static final Configuration CLUSTER_CONFIGURATION = 
getClusterConfiguration();
 
-   private static int port = -1;
+   @ClassRule
+   public static final MiniClusterResource CLUSTER = new 
MiniClusterResource(
+   new MiniClusterResource.MiniClusterResourceConfiguration(
+   CLUSTER_CONFIGURATION,
+   NUM_TASK_MANAGERS,
+   NUM_SLOTS),
+   true
+   );
 
-   @BeforeClass
-   public static void initialize() throws Exception {
+   private static Configuration getClusterConfiguration() {
Configuration config = new Configuration();
-   config.setInteger(ConfigConstants.LOCAL_NUMBER_TASK_MANAGER, 
NUM_TASK_MANAGERS);
-   config.setInteger(ConfigConstants.TASK_MANAGER_NUM_TASK_SLOTS, 
NUM_SLOTS);
-   config.setLong(TaskManagerOptions.MANAGED_MEMORY_SIZE, 12L);
-   config.setBoolean(ConfigConstants.LOCAL_START_WEBSERVER, true);
-
-   File logDir = File.createTempFile("TestBaseUtils-logdir", null);
-   assertTrue("Unable to delete temp file", logDir.delete());
-   assertTrue("Unable to create temp directory", logDir.mkdir());
-   File logFile = new File(logDir, "jobmanager.log");
-   File outFile = new File(logDir, "jobmanager.out");
-
-   Files.createFile(logFile.toPath());
-   Files.createFile(outFile.toPath());
+   try {
+   File logDir = 
File.createTempFile("TestBaseUtils-logdir", null);
--- End diff --

Making it a rule is a bit tricky since you end up with a dependency between 
rules as the cluster is also one.


---

[GitHub] flink pull request #5665: [FLINK-8703][tests] Port WebFrontendITCase to Mini...

2018-03-09 Thread aljoscha 
Github user aljoscha commented on a diff in the pull request:

https://github.com/apache/flink/pull/5665#discussion_r173436756
  
--- Diff: 
flink-runtime-web/src/test/java/org/apache/flink/runtime/webmonitor/WebFrontendITCase.java
 ---
@@ -66,40 +73,44 @@
private static final int NUM_TASK_MANAGERS = 2;
private static final int NUM_SLOTS = 4;
 
-   private static LocalFlinkMiniCluster cluster;
+   private static final Configuration CLUSTER_CONFIGURATION = 
getClusterConfiguration();
 
-   private static int port = -1;
+   @ClassRule
+   public static final MiniClusterResource CLUSTER = new 
MiniClusterResource(
+   new MiniClusterResource.MiniClusterResourceConfiguration(
+   CLUSTER_CONFIGURATION,
+   NUM_TASK_MANAGERS,
+   NUM_SLOTS),
+   true
+   );
 
-   @BeforeClass
-   public static void initialize() throws Exception {
+   private static Configuration getClusterConfiguration() {
Configuration config = new Configuration();
-   config.setInteger(ConfigConstants.LOCAL_NUMBER_TASK_MANAGER, 
NUM_TASK_MANAGERS);
-   config.setInteger(ConfigConstants.TASK_MANAGER_NUM_TASK_SLOTS, 
NUM_SLOTS);
-   config.setLong(TaskManagerOptions.MANAGED_MEMORY_SIZE, 12L);
-   config.setBoolean(ConfigConstants.LOCAL_START_WEBSERVER, true);
-
-   File logDir = File.createTempFile("TestBaseUtils-logdir", null);
-   assertTrue("Unable to delete temp file", logDir.delete());
-   assertTrue("Unable to create temp directory", logDir.mkdir());
-   File logFile = new File(logDir, "jobmanager.log");
-   File outFile = new File(logDir, "jobmanager.out");
-
-   Files.createFile(logFile.toPath());
-   Files.createFile(outFile.toPath());
+   try {
+   File logDir = 
File.createTempFile("TestBaseUtils-logdir", null);
--- End diff --

This could use the `TemporaryFolder` `@Rule`, couldn't it?


---

[GitHub] flink pull request #5665: [FLINK-8703][tests] Port WebFrontendITCase to Mini...

2018-03-09 Thread zentol 
GitHub user zentol opened a pull request:

https://github.com/apache/flink/pull/5665

 [FLINK-8703][tests] Port WebFrontendITCase to MiniClusterResource 

## What is the purpose of the change

Ports the `WebFrontendITCase` to use `MiniClusterResource`.

## Brief change log

* implement prerequisite `MiniClusterClient#listJobs()`
* modify `MiniClusterResource` to expose WebUI/REST port
* make `MiniClusterResource#CODEBASE_KEY/FLIP6_CODEBASE` public to support 
version dependent test behavior (not pretty but the alternative would be a full 
copy of the test)
* port WebFrontendITCase

## Verifying this change

Run `WebFrontendITCase` with `flip6` profile enabled/disabled.


You can merge this pull request into a Git repository by running:

$ git pull https://github.com/zentol/flink 8703_web

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/flink/pull/5665.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #5665


commit 41c5737545897506470f531394269b94ebe2ef12
Author: zentol 
Date:   2018-03-07T10:05:12Z

[FLINK-8811][tests] Implement MiniClusterClient#listJobs

commit 98f366238cddcef38fff08b14764c9120dbcccea
Author: zentol 
Date:   2018-03-07T10:14:20Z

[FLINK-8703][tests] Expose WebUI port

commit cb903f3681c808a3e5011211af6b7d174f86a23e
Author: zentol 
Date:   2018-03-07T10:14:46Z

[FLINK-8703][tests] Port WebFrontendITCase to MiniClusterResource




---