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

[Alexander Tikhonov]

Alexander, thanks a lot for the review, the lack of ‘--user’ did not break the testing on Travis-CI:
https://www.travis-ci.org/github/tarantool/tarantool/jobs/666683564


  
>Среда, 25 марта 2020, 14:38 +03:00 от Alexander Turenko <alexander.turenko at 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?
The tapped formula does not exist in brew any more, though it can’t be updated.
>
>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
Actually, I’ve meant that people who run on Mac hosts, like people in our group.
>
>> + # 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?
Right, but in real it was added because of gitlab-ci, for now we have well configured OSX like travis’s.
>
>It looks a bit strance that pip w/o --user just works from 'travis'
>user.
You may check that testing passed with installation part without issues (check from line 8908):
https://www.travis-ci.org/github/tarantool/tarantool/jobs/666683564
>
>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.
Ok, sure
>
>> + 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…
This commit is already complete of changes and the change on the testing suites better to make in the next patch, due to the list of suites were not changed.
>
>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?
‘var’ directory lays too deep at the path and the length of the path is too long for test-run.py tool which fails because of it, the solution to fix it is to use the short link for the ‘var’ directory.
>
>Anyway, it should be harmless. So kinda ok. 
 
 
--
Alexander Tikhonov
 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.tarantool.org/pipermail/tarantool-patches/attachments/20200325/55ac991a/attachment.html>