David Caro has posted comments on this change. Change subject: reorg repoclosure + tests ......................................................................
Patch Set 22: (9 comments) https://gerrit.ovirt.org/#/c/39249/22/jobs/packaging/test_repo_closure_check.sh File jobs/packaging/test_repo_closure_check.sh: Line 1: #!/usr/bin/env bats Line 2: Line 3: TEST_RUN=1 not needed Line 4: tested="repo_closure_check" Line 5: [[ -h "${tested}.bash" ]] || ln -s "${tested}.sh" "${tested}.bash" Line 6: load "${tested}" Line 7: Line 2: Line 3: TEST_RUN=1 Line 4: tested="repo_closure_check" Line 5: [[ -h "${tested}.bash" ]] || ln -s "${tested}.sh" "${tested}.bash" Line 6: load "${tested}" what is this for? Line 7: Line 8: Line 9: @test "test function: set_if_undefined" { Line 10: local varname="KSKSK" Line 45: # echo "FEDORA_MIRROR=${FEDORA_MIRROR}" >> results.out Line 46: # echo "GLUSTER_MIRROR=${GLUSTER_MIRROR}" >> results.out Line 47: # echo "JPACKAGE_MIRROR=${JPACKAGE_MIRROR}" >> results.out Line 48: # echo "COPR=${COPR}" >> results.out Line 49: [[ 0 -eq "$?" ]] if not using 'run' there's no need to check the return value Line 50: #[[ "" == "${output}" ]] Line 51: [[ "http://resources.ovirt.org"; == "${BASE_URL}" ]] Line 52: [[ "http://centos.mirror.constant.com"; == "${CENTOS_MIRROR}" ]] Line 53: [[ "http://mirror.switch.ch/ftp/mirror"; == "${EPEL_MIRROR}" ]] Line 58: unset REPO_NAME Line 59: run validation Line 60: Line 61: expected="FATAL: Please specify --repo= option" Line 62: [[ 0 -eq "${status}" ]] shouldn't it return 1? Line 63: [[ "${expected}" == "${lines[0]}" ]] Line 64: Line 65: Line 66: DISTRIBUTION="Ubuntu" Line 99: fi Line 100: done Line 101: delim="|" Line 102: processed="" Line 103: for i in ${raw[@]}; do quotes Line 104: if [ -z "${processed}" ]; then Line 105: processed="${i}" Line 106: else Line 107: processed="${processed}${delim}${i}" Line 100: done Line 101: delim="|" Line 102: processed="" Line 103: for i in ${raw[@]}; do Line 104: if [ -z "${processed}" ]; then [[ ]] 3 char vars You might want to avoid it altogether initializing processed: processed="${raw[0]}" for elem in "${raw[@]:1}"; do ... Line 105: processed="${i}" Line 106: else Line 107: processed="${processed}${delim}${i}" Line 108: fi Line 103: for i in ${raw[@]}; do Line 104: if [ -z "${processed}" ]; then Line 105: processed="${i}" Line 106: else Line 107: processed="${processed}${delim}${i}" this can be replaced with: processed+="${delim}${i}" Line 108: fi Line 109: done Line 110: run repo_data_to_arr "${delim}" "${processed}" Line 111: [[ 0 -eq "${status}" ]] Line 141: extras="exclude=mesdb*" Line 142: run repo_data_to_conf_section "${rpath}" "${repoid}" "${field}" "${value}" "${extras}" Line 143: [[ 0 -eq "${status}" ]] Line 144: [[ "" == "${output}" ]] Line 145: run diff ${rpath_fixture} ${rpath} &> /dev/null no need to redirect Line 146: [[ 0 -eq "${status}" ]] Line 147: rm -f "${rpath}" Line 148: } Line 149: Line 179: "kuku,mirrorlist,http://kuku.com/alsdkas/kasd,include=libsbonazzo*"; Line 180: "kuakua,baseurl,http://kuakua.com/alsdkas/kasd/jdjdj"; Line 181: ) Line 182: Line 183: run append_repos_to_rpath ${rpath} ${delim} ${repos[@]} quotes Line 184: [[ "" == "${output}" ]] Line 185: run diff "${rpath_fixture}" "${rpath}" &> /dev/null Line 186: [[ 0 -eq "${status}" ]] Line 187: rm -fr "${rpath}" -- To view, visit https://gerrit.ovirt.org/39249 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I207c5c59ca3149c17897c231e391e15e7eec4363 Gerrit-PatchSet: 22 Gerrit-Project: jenkins Gerrit-Branch: master Gerrit-Owner: Max Kovgan <m...@redhat.com> Gerrit-Reviewer: David Caro <dcaro...@redhat.com> Gerrit-Reviewer: Max Kovgan <m...@redhat.com> Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: Sandro Bonazzola <sbona...@redhat.com> Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
