Skip to content

Commit

Permalink
[Improvement][Test] Remove powermock in service and server modules (a…
Browse files Browse the repository at this point in the history 
…pache#12164)

* Remove powermock and refactor some related code in dolphinscheduler-service and dolphinscheduler-server modules
* Remove redundant comments
* Add null check
  • Loading branch information
@EricGao888
EricGao888 authored and fuchanghai committed Oct 17, 2022
1 parent a8f5977 commit a2ca5d9
Show file tree
Hide file tree
Showing 12 changed files with 261 additions and 193 deletions.
53 changes: 29 additions & 24 deletions dolphinscheduler-server/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
~ See the License for the specific language governing permissions and
~ limitations under the License.
-->

<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>
Expand All @@ -25,8 +24,20 @@
<version>dev-SNAPSHOT</version>
</parent>
<artifactId>dolphinscheduler-server</artifactId>
<name>dolphinscheduler-server</name>
<packaging>jar</packaging>
<name>dolphinscheduler-server</name>

<dependencyManagement>
<dependencies>
<dependency>
<groupId>org.apache.dolphinscheduler</groupId>
<artifactId>dolphinscheduler-bom</artifactId>
<version>${project.version}</version>
<type>pom</type>
<scope>import</scope>
</dependency>
</dependencies>
</dependencyManagement>

<dependencies>
<!-- dolphinscheduler -->
Expand All @@ -47,7 +58,13 @@
<artifactId>mockito-core</artifactId>
<scope>test</scope>
</dependency>

<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-inline</artifactId>
<version>3.12.4</version>
<!-- TODO: move this dependency to root pom after removing powermock in the whole project -->
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.apache.hadoop</groupId>
<artifactId>hadoop-common</artifactId>
Expand All @@ -62,8 +79,8 @@
<artifactId>jdk.tools</artifactId>
</exclusion>
<exclusion>
<artifactId>servlet-api</artifactId>
<groupId>javax.servlet</groupId>
<artifactId>servlet-api</artifactId>
</exclusion>
<exclusion>
<groupId>javax.servlet</groupId>
Expand Down Expand Up @@ -127,16 +144,16 @@
<artifactId>jsp-api</artifactId>
</exclusion>
<exclusion>
<artifactId>jersey-json</artifactId>
<groupId>com.sun.jersey</groupId>
<artifactId>jersey-json</artifactId>
</exclusion>
<exclusion>
<artifactId>jersey-server</artifactId>
<groupId>com.sun.jersey</groupId>
<artifactId>jersey-server</artifactId>
</exclusion>
<exclusion>
<artifactId>jersey-core</artifactId>
<groupId>com.sun.jersey</groupId>
<artifactId>jersey-core</artifactId>
</exclusion>
</exclusions>
</dependency>
Expand All @@ -154,8 +171,8 @@
<artifactId>slf4j-log4j12</artifactId>
</exclusion>
<exclusion>
<artifactId>servlet-api</artifactId>
<groupId>javax.servlet</groupId>
<artifactId>servlet-api</artifactId>
</exclusion>
<exclusion>
<groupId>org.codehaus.jackson</groupId>
Expand All @@ -179,37 +196,25 @@
<artifactId>hadoop-mapreduce-client-shuffle</artifactId>
</exclusion>
<exclusion>
<artifactId>jersey-client</artifactId>
<groupId>com.sun.jersey</groupId>
<artifactId>jersey-client</artifactId>
</exclusion>
<exclusion>
<artifactId>jersey-core</artifactId>
<groupId>com.sun.jersey</groupId>
<artifactId>jersey-core</artifactId>
</exclusion>
<exclusion>
<artifactId>jaxb-api</artifactId>
<groupId>javax.xml.bind</groupId>
<artifactId>jaxb-api</artifactId>
</exclusion>
<exclusion>
<artifactId>log4j</artifactId>
<groupId>log4j</groupId>
<artifactId>log4j</artifactId>
</exclusion>
</exclusions>
</dependency>
</dependencies>

<dependencyManagement>
<dependencies>
<dependency>
<groupId>org.apache.dolphinscheduler</groupId>
<artifactId>dolphinscheduler-bom</artifactId>
<version>${project.version}</version>
<type>pom</type>
<scope>import</scope>
</dependency>
</dependencies>
</dependencyManagement>

<build>
<plugins>
<plugin>
Expand Down
28 changes: 18 additions & 10 deletions ...scheduler-server/src/main/java/org/apache/dolphinscheduler/server/utils/ProcessUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,6 @@

package org.apache.dolphinscheduler.server.utils;

import lombok.NonNull;
import org.apache.commons.collections.CollectionUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.SystemUtils;
import org.apache.dolphinscheduler.common.Constants;
import org.apache.dolphinscheduler.common.utils.CommonUtils;
import org.apache.dolphinscheduler.common.utils.FileUtils;
Expand All @@ -31,10 +27,11 @@
import org.apache.dolphinscheduler.plugin.task.api.enums.TaskExecutionStatus;
import org.apache.dolphinscheduler.remote.utils.Host;
import org.apache.dolphinscheduler.service.log.LogClient;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.annotation.Nullable;
import org.apache.commons.collections.CollectionUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.SystemUtils;

import java.io.File;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
Expand All @@ -43,6 +40,13 @@
import java.util.regex.Matcher;
import java.util.regex.Pattern;

import javax.annotation.Nullable;

import lombok.NonNull;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* mainly used to get the start command line of a process.
*/
Expand Down Expand Up @@ -166,7 +170,9 @@ public static String getPidsStr(int processId) throws Exception {
}
} else {
String pids = OSUtils.exeCmd(String.format("%s -p %d", Constants.PSTREE, processId));
mat = WINDOWSATTERN.matcher(pids);
if (null != pids) {
mat = WINDOWSATTERN.matcher(pids);
}
}

if (null != mat) {
Expand Down Expand Up @@ -206,10 +212,12 @@ public static String getPidsStr(int processId) throws Exception {
taskExecutionContext.getTaskInstanceId()));
}
FileUtils.createWorkDirIfAbsent(taskExecutionContext.getExecutePath());
cancelApplication(appIds, logger, taskExecutionContext.getTenantCode(), taskExecutionContext.getExecutePath());
cancelApplication(appIds, logger, taskExecutionContext.getTenantCode(),
taskExecutionContext.getExecutePath());
return appIds;
} else {
logger.info("The current appId is empty, don't need to kill the yarn job, taskInstanceId: {}", taskExecutionContext.getTaskInstanceId());
logger.info("The current appId is empty, don't need to kill the yarn job, taskInstanceId: {}",
taskExecutionContext.getTaskInstanceId());
}
} catch (Exception e) {
logger.error("Kill yarn job failure, taskInstanceId: {}", taskExecutionContext.getTaskInstanceId(), e);
Expand Down
47 changes: 19 additions & 28 deletions ...duler-server/src/test/java/org/apache/dolphinscheduler/server/utils/ProcessUtilsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,31 +17,28 @@

package org.apache.dolphinscheduler.server.utils;

import static org.powermock.api.mockito.PowerMockito.when;
import org.apache.commons.lang3.SystemUtils;
import static org.mockito.ArgumentMatchers.anyString;

import org.apache.dolphinscheduler.common.Constants;
import org.apache.dolphinscheduler.common.utils.HadoopUtils;
import org.apache.dolphinscheduler.common.utils.OSUtils;
import org.apache.dolphinscheduler.common.utils.PropertyUtils;
import org.apache.dolphinscheduler.plugin.task.api.enums.TaskExecutionStatus;

import java.util.ArrayList;
import java.util.List;

import org.apache.dolphinscheduler.plugin.task.api.enums.TaskExecutionStatus;
import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.mockito.Mockito;
import org.mockito.MockitoAnnotations;
import org.powermock.api.mockito.PowerMockito;
import org.powermock.core.classloader.annotations.PrepareForTest;
import org.powermock.modules.junit4.PowerMockRunner;
import org.powermock.reflect.Whitebox;
import org.mockito.junit.MockitoJUnitRunner;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

@RunWith(PowerMockRunner.class)
@PrepareForTest({System.class, OSUtils.class, HadoopUtils.class, PropertyUtils.class, SystemUtils.class})
@RunWith(MockitoJUnitRunner.class)
public class ProcessUtilsTest {

private static final Logger logger = LoggerFactory.getLogger(ProcessUtils.class);
Expand All @@ -54,23 +51,22 @@ public void setUp() {
@Test
public void getPidsStr() throws Exception {
int processId = 1;
PowerMockito.mockStatic(OSUtils.class);
Whitebox.setInternalState(SystemUtils.class, "IS_OS_MAC", true);
when(OSUtils.exeCmd(String.format("%s -p %d", Constants.PSTREE, processId))).thenReturn(null);
String pidListMac = ProcessUtils.getPidsStr(processId);
Assert.assertEquals("", pidListMac);
Mockito.mockStatic(OSUtils.class);
Mockito.when(OSUtils.exeCmd(anyString())).thenReturn(null);
String pidList = ProcessUtils.getPidsStr(processId);
Assert.assertEquals("", pidList);
}

@Test
public void testGetKerberosInitCommand() {
PowerMockito.mockStatic(PropertyUtils.class);
PowerMockito.when(PropertyUtils.getBoolean(Constants.HADOOP_SECURITY_AUTHENTICATION_STARTUP_STATE, false))
Mockito.mockStatic(PropertyUtils.class);
Mockito.when(PropertyUtils.getBoolean(Constants.HADOOP_SECURITY_AUTHENTICATION_STARTUP_STATE, false))
.thenReturn(true);
PowerMockito.when(PropertyUtils.getString(Constants.JAVA_SECURITY_KRB5_CONF_PATH)).thenReturn("/etc/krb5.conf");
PowerMockito.when(PropertyUtils.getString(Constants.LOGIN_USER_KEY_TAB_PATH)).thenReturn("/etc/krb5.keytab");
PowerMockito.when(PropertyUtils.getString(Constants.LOGIN_USER_KEY_TAB_USERNAME)).thenReturn("[email protected]");
Mockito.when(PropertyUtils.getString(Constants.JAVA_SECURITY_KRB5_CONF_PATH)).thenReturn("/etc/krb5.conf");
Mockito.when(PropertyUtils.getString(Constants.LOGIN_USER_KEY_TAB_PATH)).thenReturn("/etc/krb5.keytab");
Mockito.when(PropertyUtils.getString(Constants.LOGIN_USER_KEY_TAB_USERNAME)).thenReturn("[email protected]");
Assert.assertNotEquals("", ProcessUtils.getKerberosInitCommand());
PowerMockito.when(PropertyUtils.getBoolean(Constants.HADOOP_SECURITY_AUTHENTICATION_STARTUP_STATE, false))
Mockito.when(PropertyUtils.getBoolean(Constants.HADOOP_SECURITY_AUTHENTICATION_STARTUP_STATE, false))
.thenReturn(false);
Assert.assertEquals("", ProcessUtils.getKerberosInitCommand());
}
Expand All @@ -84,17 +80,12 @@ public void testCancelApplication() {
String executePath = "/ds-exec/1/1/1";
TaskExecutionStatus running = TaskExecutionStatus.RUNNING_EXECUTION;

PowerMockito.mockStatic(HadoopUtils.class);
Mockito.mockStatic(HadoopUtils.class);
HadoopUtils hadoop = HadoopUtils.getInstance();

try {
PowerMockito.whenNew(HadoopUtils.class).withAnyArguments().thenReturn(hadoop);
} catch (Exception e) {
e.printStackTrace();
}
try {
when(hadoop.getApplicationStatus("application_1585532379175_228491")).thenReturn(running);
when(hadoop.getApplicationStatus("application_1598885606600_3677")).thenReturn(running);
Mockito.when(hadoop.getApplicationStatus("application_1585532379175_228491")).thenReturn(running);
Mockito.when(hadoop.getApplicationStatus("application_1598885606600_3677")).thenReturn(running);
} catch (Exception e) {
e.printStackTrace();
ProcessUtils.cancelApplication(appIds, logger, tenantCode, executePath);
Expand Down
36 changes: 22 additions & 14 deletions dolphinscheduler-service/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,29 @@
-->
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>
<parent>
<artifactId>dolphinscheduler</artifactId>
<groupId>org.apache.dolphinscheduler</groupId>
<artifactId>dolphinscheduler</artifactId>
<version>dev-SNAPSHOT</version>
</parent>
<modelVersion>4.0.0</modelVersion>

<artifactId>dolphinscheduler-service</artifactId>

<name>dolphinscheduler-service</name>

<dependencyManagement>
<dependencies>
<dependency>
<groupId>org.apache.dolphinscheduler</groupId>
<artifactId>dolphinscheduler-bom</artifactId>
<version>${project.version}</version>
<type>pom</type>
<scope>import</scope>
</dependency>
</dependencies>
</dependencyManagement>

<dependencies>
<!-- dolphinscheduler -->
<dependency>
Expand All @@ -51,6 +63,14 @@
<artifactId>dolphinscheduler-task-api</artifactId>
</dependency>

<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-inline</artifactId>
<version>3.12.4</version>
<!-- TODO: move this dependency to root pom after removing powermock in the whole project -->
<scope>test</scope>
</dependency>

<dependency>
<groupId>com.cronutils</groupId>
<artifactId>cron-utils</artifactId>
Expand All @@ -63,16 +83,4 @@
</dependency>

</dependencies>

<dependencyManagement>
<dependencies>
<dependency>
<groupId>org.apache.dolphinscheduler</groupId>
<artifactId>dolphinscheduler-bom</artifactId>
<version>${project.version}</version>
<type>pom</type>
<scope>import</scope>
</dependency>
</dependencies>
</dependencyManagement>
</project>
Loading

0 comments on commit a2ca5d9

Please sign in to comment.