Re: [Tarantool-patches] [PATCH v2] test: fix OSX host setup

[Alexander Turenko <alexander.turenko@tarantool.org>]

If lack of --user (for pip) does not fail Travis CI jobs, then the whole
commit should be harmless. I cannot say that I'm happy with all changes,
but let's don't freeze on me: fix what you personally think should be
fixed and go to Kirill to push. No need to re-review with me.

WBR, Alexander Turenko.

On Mon, Mar 23, 2020 at 09:09:05PM +0300, Alexander V. Tikhonov wrote:
> Fixed OSX host setup for Tarantool build:
> - set brew installation from Homebrew repository instructions;
> - set in use Python 2 latest commit from tapped local formula,
>   since Python 2 is EOL, also removed extra pip installation with
>   get-pip script, because tapped formula installs pip itself;
> - added upgrade packages call to avoid of fails on already
>   installed packages, but with previous version.
> 
> Fixed OSX host setup for Tarantool test:
> - set maximum processes limit value to 2500 for testing process;
> - set 'var' directory for test-run tool to the shorter path name
>   to avoid of issues with long names;
> ---
> 
> Github: https://github.com/tarantool/tarantool/tree/avtikhon/osx_depends-full-ci
> Based on 1st commit from Github: https://github.com/tarantool/tarantool/tree/avtikhon/osx_on_mini14-full-ci
> Which already has LGTM:
>     https://lists.tarantool.org/pipermail/tarantool-patches/2020-March/014734.html

> +	brew update || echo | /usr/bin/ruby -e \
> +		"$$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/master/install)"

Why don't update formulae after installing brew?

From the previous review [1]:

> > > So you added brew installation when it is not installed. Add this to the
> > > commit message at least.
> >
> > Is it possible at all for a machine that is enabled into CI? Aren't you
> > use brew to install gitlab runner?
> >
> > I’ve added the commit message about it. This change is really needed to be sure that
> > our customers can use this script too.

Are there any customer who run our testing? I don't know anyone. Even if
one doing it, (s)he prepares an environment and invoke `make test` or
`cd test && ./test-run.py`.

So it is the dead code. Personally I prefer to don't have dead code. If
you decided to do it, okay.

[1]: https://lists.tarantool.org/pipermail/tarantool-patches/2020-March/015062.html

> +	# try to install the packages either upgrade it to avoid of fails
> +	# if the package already exists with the previous version
> +	brew install --force ${OSX_PKGS} || brew upgrade ${OSX_PKGS}
> +	pip install --force-reinstall -r test-run/requirements.txt

--user (pip option) is not needed anymore? It was added to avoid using
of sudo or virtualenv. Is it related to a Travis CI infrastructure
change?

It looks a bit strance that pip w/o --user just works from 'travis'
user.

Please, don't do such subtle changes. At least describe a reason in the
commit message. Otherwise it will be nightmare in the future, when one
will try to understand what is made for what, what is actual, what can
be simplified / removed / refactored.

> +		cd test && ./test-run.py --vardir /tmp/tnt --force $(TEST_RUN_EXTRA_PARAMS) \
>  			app/ app-tap/ box/ box-py/ box-tap/ engine/ engine_long/ long_run-py/ luajit-tap/ \
>  			replication-py/ small/ sql/ sql-tap/ swim/ unit/ vinyl/ wal_off/ xlog/ xlog-py/

Why not to use --exclude or TEST_RUN_EXCLUDE?

Are there any issue about disabled tests?

Why the whole suite is going to be disabled? To save time to investigate
problems?

When we'll investigate and enable tests rather then only disable them?
The past year trend to don't enable anything back...

From the previous review [1]:

> > The problem with 'var' directory is not related to the new way to Python
> > 2 installation? Why everything is in one commit?
>
> This commit is about OSX host setup for building and testing, so I
> think it is ok to set it here.

It sounds as 'because something going not right, when something was
changed'. Neat reason.

I cannot say neither 'ok', not 'not ok' for this. Okay, I can guess that
working directory is changed somehow, but why and where?

Anyway, it should be harmless. So kinda ok.