Re: [libvirt PATCH 7/7] ci: integration.sh: Define the SCRATCH_DIR variable for local execution
On Mon, Jan 23, 2023 at 09:41:10AM +0100, Martin Kletzander wrote:
On Fri, Jan 20, 2023 at 04:06:03PM +0100, Erik Skultety wrote:
> Running outside of GitLab will likely not have the variable set and
> hence the execution would fail.
>
> Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
> ---
> ci/integration.sh | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/ci/integration.sh b/ci/integration.sh
> index 41326d6e40..ac04c46d8e 100644
> --- a/ci/integration.sh
> +++ b/ci/integration.sh
> @@ -37,6 +37,14 @@ virsh net-start default &>/dev/null || true
> # END AS ROOT
> exit
>
> +# If we're running outside of GitLab, this variable will likely not exist, so
> +# we need to define it and create the scratch directory
> +if [ -z "$SCRATCH_DIR" ]
> +then
> + SCRATCH_DIR="/tmp/scratch"
> + mkdir "$SCRATCH_DIR" 2>/dev/null
This could fail if someone has this directory already. Which is a good
thing as otherwise it could override some of it. But wouldn't it be
nicer to use mktemp -d and print the result?
Although an option, the main motivation here to remain consistent with how it
works in GitLab. Since we define SCRATCH_DIR under the 'vars' section, IIRC you
can only use a scalar value, not a command (if we can, I retract my argument)
and hence we'd have to export and define the variable under each script,
before_script, after_script sections.
So, I preferred consistency here, since I wouldn't realistically expect an
engineer to have /tmp/scratch prior to executing script given the main
motivation here is to quickly get a throwaway test machine to run the script
and THEN debug if the tests fail. So, while I agree you're right here, I don't
think it's a likely scenario.
Erik