From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 6B600469719 for ; Thu, 24 Sep 2020 07:24:11 +0300 (MSK) Date: Thu, 24 Sep 2020 07:24:09 +0300 From: "Alexander V. Tikhonov" Message-ID: <20200924042409.GA3997@hpalx> References: <20200924034031.htguqxtq3svnx2om@tkn_work_nb> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20200924034031.htguqxtq3svnx2om@tkn_work_nb> Subject: Re: [Tarantool-patches] [PATCH v3] gitlab-ci: save failed test results artifacts List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Turenko Cc: tarantool-patches@dev.tarantool.org Hi Alexander, thanks for the review, just commented your questions to make it clear. On Thu, Sep 24, 2020 at 06:40:31AM +0300, Alexander Turenko wrote: > Nothing contradicts with my vision (I became a bit more far from details > now). LGTM. Several comments below (no need to re-review with me if > you'll want to make changes according to those comments). > > WBR, Alexander Turenko. > > > .vbox_template: &vbox_definition > > stage: test > > after_script: > > - - ${GITLAB_MAKE} vms_shutdown > > + - > > > + scp -r -P ${VMS_PORT} ${VMS_USER}@127.0.0.1:tarantool/test/var/artifacts . ; > > + ${GITLAB_MAKE} vms_shutdown > > + <<: *artifacts_files_definition > > Nit: Using literal style indicator (the pipe symbol instead of the angle > bracket) would look more intuitive and does not require the semicolon. > (The folded style ('>') replaces newlines with spaces and it should be > used when this behaviour is wanted I think.) > I did use it after investigation and found the following, [1] - where in the gitlab-ci issue disscussion was told to use '>' instead of '|', and previous discussion [2], where it was described. [1] - https://gitlab.com/gitlab-org/gitlab-runner/-/issues/166 [2] - https://stackoverflow.com/questions/3790454/how-do-i-break-a-string-over-multiple-lines/3790497#3790497 > > static_build_cmake_osx_15: > > - stage: test > > + <<: *osx_definition > > tags: > > - osx_15 > > script: > > Nit: This change is not mentioned in the commit message. > That is absolutely the same code. It was used to inherit 'after_script' part: .osx_template: &osx_definition <<: *artifacts_files_definition stage: test script: - ${GITLAB_MAKE} test_osx after_script: # Artifacts can't be used from outside the project directory, check: # https://docs.gitlab.com/ee/ci/yaml/README.html#artifactspaths # "Paths are relative to the project directory ($CI_PROJECT_DIR)" # "and can’t directly link outside it." - cp -rf ${OSX_VARDIR}/artifacts . 'script' part would be overwritten from '&osx_definition': script: - ${GITLAB_MAKE} test_osx to: script: - ${GITLAB_MAKE} test_static_build_cmake_osx > > diff --git a/.travis.mk b/.travis.mk > > index d8f97ef5c..577970d8f 100644 > > --- a/.travis.mk > > +++ b/.travis.mk > > @@ -242,11 +242,11 @@ INIT_TEST_ENV_OSX=\ > > launchctl limit maxproc || : ; \ > > ulimit -u ${MAX_PROC} || : ; \ > > ulimit -u ; \ > > - rm -rf /tmp/tnt > > + rm -rf ${OSX_VARDIR} > > Just in case: should not we set some default value for this variable in > case we'll run this rule outside of CI (or in CI, which is not GitLab > CI)? Right, it' better to have, corrected .gitlab.mk: diff --git a/.travis.mk b/.travis.mk index 577970d8f..c09f1c4cb 100644 --- a/.travis.mk +++ b/.travis.mk @@ -9,6 +9,7 @@ MAX_FILES?=65534 MAX_PROC?=2500 OOS_SRC_PATH="/source" BIN_DIR=/usr/local/bin +OSX_VARDIR?=/tmp/tnt CLOJURE_URL="https://download.clojure.org/install/linux-install-1.10.1.561.sh" LEIN_URL="https://raw.githubusercontent.com/technomancy/leiningen/stable/bin/lein"