Среда, 25 марта 2020, 15:30 +03:00 от Alexander Turenko <alexander.turenko@tarantool.org>:
 

> >> + 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.

After installing brew you'll have fresh brew, but no formulae, I guess.
Will the next `brew install` work? I guess, no. You need `brew update`
after this.

So it seems logical to do `[ -z "$(which brew) "] && ruby
brew_install_script; brew update` rather than `brew update || ruby
brew_install_script`.

> >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.

Developers doing `make test` or `cd test && ./test-run.py`. Nobody want
to run a testing script and found that something was changed in its
system. So I'm still on my view.

> >> + # 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' -- the option? Okay so. Good piece of information to add to the
commit message.

> >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

Okay. Yep, it seems pip assumes --user by default in Travis CI. Maybe I
wrongly remember that --user was added for Travis CI (AFAIR it was done
by Arseniy).

> >> + 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.

Let's add the info about Mac OS X testing into the relevant issue to
track disabled tests (AFAIR you have one, but it was about testing
during RPM packages build).

> >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.

Why it was okay, but becomes too long?