David Caro has posted comments on this change. Change subject: Adding system tests ......................................................................
Patch Set 19: (9 comments) minor bash stuff and a suggestion on the archiving https://gerrit.ovirt.org/#/c/37069/19/jobs/confs/shell-scripts/system_tests.cleanup.sh File jobs/confs/shell-scripts/system_tests.cleanup.sh: Line 3: WORKSPACE? use ${var:?} instead, that will make sure that the var is not null and defined https://gerrit.ovirt.org/#/c/37069/19/jobs/confs/shell-scripts/system_tests.collect_logs.sh File jobs/confs/shell-scripts/system_tests.collect_logs.sh: Line 3: E? same as before Line 5: WORKSPACE? store this on a local (to the script) variable, no need to recheck the var each time: workdir="${WORKSPACE:?}" echo "$workdir" ... If you don't want to create a new var, do this at the top of the script, helps seeing which vars it needs and allows you to ignore all the extra :? WORKSPACE="${WORKSPACE:?}" BUILD_NUMBER="${BUILD_NUMBER:?}" Line 28: r no need for this dangerous -r here Line 35: rm -rf "${{PREFIX?}}" Line 36: Line 37: cd "${{WORKSPACE?}}/" Line 38: tar cvzf \ Line 39: "exported-archives.tar.gz" \ not sure what is this for, better to leave it as a dir as we do on all the jobs and, if you want to, create a tar of the contents inside that dir instead. also, gzipping without removing the source duplicates space (if you archive both, that is) Line 40: "exported-archives/" https://gerrit.ovirt.org/#/c/37069/19/jobs/confs/shell-scripts/system_tests_common.sh File jobs/confs/shell-scripts/system_tests_common.sh: Line 20: DISTRO no need for the check Line 24: WORKSPACE add this to the vars at the top, like version https://gerrit.ovirt.org/#/c/37069/19/jobs/confs/yaml/templates/system_tests.yaml File jobs/confs/yaml/templates/system_tests.yaml: Line 30: - log-text: '.*' Line 31: operator: AND Line 32: script: !include-raw shell-scripts/system_tests.collect_logs.sh Line 33: - archive: Line 34: artifacts: 'exported-archives.tar.gz' I recommend gzipping each of the elements on that dir per separate and achiving everything under the dir, it's not the same downloading just one log to debug than having to download the whole lot Line 35: allow-empty: true Line 36: - email: Line 37: recipients: '{email-to}' Line 38: notify-every-unstable-build: false # FIXME https://gerrit.ovirt.org/#/c/37069/19/jobs/confs/yaml/triggers/gerrit.yaml File jobs/confs/yaml/triggers/gerrit.yaml: Line 133: triggers: Line 134: - gerrit: Line 135: trigger-on: Line 136: - comment-added-event: Line 137: approval-category: 'CRVW' this might change to the workflow flag sooner than later. Line 138: approval-value: 2 Line 139: escape-quotes: true Line 140: projects: Line 141: - project-compare-type: 'PLAIN' -- To view, visit https://gerrit.ovirt.org/37069 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If411d80e4213f5d4a5d85defeb6bb1634f9bcae4 Gerrit-PatchSet: 19 Gerrit-Project: jenkins Gerrit-Branch: master Gerrit-Owner: David Caro <[email protected]> Gerrit-Reviewer: David Caro <[email protected]> Gerrit-Reviewer: Dima Kuznetsov <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
