From 346705967d790b65fd66181570e136a6a0d59bd3 Mon Sep 17 00:00:00 2001 From: zhouwei25 <52485244+zhouwei25@users.noreply.github.com> Date: Fri, 6 Dec 2019 18:59:14 +0800 Subject: [PATCH] monitoring changes of unittest, delete one unittest will need approve (#21377) --- paddle/scripts/paddle_build.sh | 56 ++++++++++++++++++++++++++++++++++ tools/check_api_approvals.sh | 2 +- tools/diff_unittest.py | 31 +++++++++++++++++++ 3 files changed, 88 insertions(+), 1 deletion(-) create mode 100644 tools/diff_unittest.py diff --git a/paddle/scripts/paddle_build.sh b/paddle/scripts/paddle_build.sh index ac569a3c7a5..1315771bdf5 100755 --- a/paddle/scripts/paddle_build.sh +++ b/paddle/scripts/paddle_build.sh @@ -506,6 +506,57 @@ function generate_api_spec() { deactivate } +function check_change_of_unittest() { + fetch_upstream_develop_if_not_exist + cur_branch=`git branch | grep \* | cut -d ' ' -f2` + git checkout -b develop_base_pr upstream/$BRANCH + cmake_gen $1 + generate_unittest_spec "DEV" + git checkout $cur_branch + git branch -D develop_base_pr + rm -rf ${PADDLE_ROOT}/build + cmake_gen $1 + generate_unittest_spec "PR" + + # approval_user_list: XiaoguangHu01 46782768,luotao1 6836917,phlrain 43953930,lanxianghit 47554610, zhouwei25 52485244 + approval_line=`curl -H "Authorization: token ${GITHUB_API_TOKEN}" https://api.github.com/repos/PaddlePaddle/Paddle/pulls/${GIT_PR_ID}/reviews?per_page=10000` + tests_spec_diff=`python ${PADDLE_ROOT}/tools/diff_unittest.py ${PADDLE_ROOT}/paddle/fluid/UNITTEST_DEV.spec ${PADDLE_ROOT}/paddle/fluid/UNITTEST_PR.spec` + if [ "$test_spec_diff" != "" ]; then + APPROVALS=`echo ${approval_line}|python ${PADDLE_ROOT}/tools/check_pr_approval.py 1 46782768 6836917 43953930 47554610` + echo "current pr ${GIT_PR_ID} got approvals: ${APPROVALS}" + if [ "${APPROVALS}" == "FALSE" ]; then + echo "****************" + echo -e "You must have one RD (luotao1 or XiaoguangHu01 or phlrain or lanxianghit or zhouwei25) approval for the deletion of unit tests.\n" + echo "There are one approved errors." + echo "****************" + exit 1 + fi + fi + ENABLE_MAKE_CLEAN="OFF" +} + +function generate_unittest_spec() { + spec_kind=$1 + if [ "$spec_kind" == "DEV" ]; then + cat < ${spec_path} +} + function assert_api_spec_approvals() { /bin/bash ${PADDLE_ROOT}/tools/check_api_approvals.sh @@ -1111,6 +1162,11 @@ function main() { build_mac run_mac_test ${PYTHON_ABI:-""} ${PROC_RUN:-1} ;; + maccheck_py35) + check_change_of_unittest ${PYTHON_ABI:-""} + build_mac + run_mac_test ${PYTHON_ABI:-""} ${PROC_RUN:-1} + ;; macbuild) cmake_gen ${PYTHON_ABI:-""} build_mac diff --git a/tools/check_api_approvals.sh b/tools/check_api_approvals.sh index 8c7518ed517..e7fdca27a8d 100644 --- a/tools/check_api_approvals.sh +++ b/tools/check_api_approvals.sh @@ -78,7 +78,7 @@ for API_FILE in ${API_FILES[*]}; do echo "checking ${API_FILE} change, PR: ${GIT_PR_ID}, changes: ${API_CHANGE}" if [ "${API_CHANGE}" ] && [ "${GIT_PR_ID}" != "" ]; then # NOTE: per_page=10000 should be ok for all cases, a PR review > 10000 is not human readable.q - # approval_user_list: XiaoguangHu01 46782768,Xreki 12538138,luotao1 6836917,sneaxiy 32832641,qingqing01 7845005,guoshengCS 14105589,heavengate 12605721,kuke 3064195,Superjomn 328693,lanxianghit 47554610,cyj1986 39645414,hutuxian 11195205,frankwhzhang 20274488,nepeplwu 45024560,Dianhai 38231817,JiabinYang 22361972,chenwhql 22561442,zhiqiu 6888866,seiriosPlus 5442383,gongweibao 10721757,saxon-zh 2870059,Boyan-Liu 2870059, Aurelius84 9301846, liym27 33742067, zhhsplendid 7913861. + # approval_user_list: XiaoguangHu01 46782768,Xreki 12538138,luotao1 6836917,sneaxiy 32832641,qingqing01 7845005,guoshengCS 14105589,heavengate 12605721,kuke 3064195,Superjomn 328693,lanxianghit 47554610,cyj1986 39645414,hutuxian 11195205,frankwhzhang 20274488,nepeplwu 45024560,Dianhai 38231817,JiabinYang 22361972,chenwhql 22561442,zhiqiu 6888866,seiriosPlus 5442383,gongweibao 10721757,saxon-zh 2870059,Boyan-Liu 2870059, zhouwei25 52485244, Aurelius84 9301846, liym27 33742067, zhhsplendid 7913861. if [ "${API_FILE}" == "paddle/fluid/op_use_default_grad_op_maker.spec" ];then echo_line="You must have one RD (sneaxiy (Recommend) or luotao1) approval for op_use_default_grad_op_maker.spec, which manages the grad_op memory optimization.\n" check_approval 1 32832641 6836917 diff --git a/tools/diff_unittest.py b/tools/diff_unittest.py new file mode 100644 index 00000000000..863e2beabfe --- /dev/null +++ b/tools/diff_unittest.py @@ -0,0 +1,31 @@ +#!/usr/bin/env python +import difflib +import sys + +with open(sys.argv[1], 'r') as f: + origin = f.read() + origin = origin.splitlines() + +with open(sys.argv[2], 'r') as f: + new = f.read() + new = new.splitlines() + +differ = difflib.Differ() +result = differ.compare(origin, new) + +error = False +diffs = [] +for each_diff in result: + if each_diff[0] == '-': # delete unit test is not allowed + error = True + diffs.append(each_diff) +''' +If you delete the unit test, such as commenting it out, +please ask for approval of one RD below for passing CI: + + - XiaoguangHu01 or luotao1 or phlrain or lanxianghit or zhouwei25 +''' +if error: + print('Deleted Unit test is: ') + for each_diff in diffs: + print(each_diff) -- GitLab