Skip to content

bump tarantool-python version #266

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 16, 2021
Merged

bump tarantool-python version #266

merged 1 commit into from
Feb 16, 2021

Conversation

ligurio
Copy link
Member

@ligurio ligurio commented Feb 10, 2021

commit daff045 ('Add luacheck config and fix warnings found by luacheck')
fixes luacheck warnings in Lua source code. Bumping version
allows to remove test-run directory from exclusions in .luacheckrc in
tarantool repository.

commit daff045 ('Add luacheck config and fix warnings found by luacheck')
fixes luacheck warnings in Lua source code. Bumping version
allows to remove test-run directory from exclusions in .luacheckrc in
tarantool repository.
@ligurio ligurio force-pushed the bump_tarantool_python branch from 2850537 to 0a2cf1f Compare February 10, 2021 11:11
@ligurio ligurio requested a review from Totktonada February 10, 2021 11:11
Copy link
Member

@Totktonada Totktonada left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@Totktonada Totktonada merged commit e565368 into master Feb 16, 2021
@Totktonada Totktonada deleted the bump_tarantool_python branch February 16, 2021 09:33
@Totktonada
Copy link
Member

test-run still has tarantool-python submodule, which has test-run submodule (older commit) and, on some level of this nesting, there are Lua files, which luacheck does not like. So I'll postpone test-run submodule update in tarantool until we'll finally remove test-run submodule from tarantool-python (see relevant tarantool/tarantool-python#111).

I verified it this way:

diff --git a/.luacheckrc b/.luacheckrc
index b75d89a0c..eae210bcf 100644
--- a/.luacheckrc
+++ b/.luacheckrc
@@ -30,7 +30,6 @@ include_files = {
 
 exclude_files = {
     "build/**/*.lua",
-    "test-run/**/*.lua",
     "test/app/*.test.lua",
     "test/box/*.test.lua",
     "test/engine/*.test.lua",
diff --git a/test-run b/test-run
index 72f8b8933..e5653684b 160000
--- a/test-run
+++ b/test-run
@@ -1 +1 @@
-Subproject commit 72f8b89332b552cf20b2143e6dae7fe5042a9f79
+Subproject commit e5653684b614cbc102ff8c643fe72745a7a3a395

make luacheck shows the following warnings after applying this patch on top of tarantool's master (2.8.0-65-g4299daf7d, t-3 is my project root, the build is in-source):

Checking /home/alex/projects/tarantool-meta/r/t-3/test-run/lib/tarantool-python/test-run/lib/tarantool-python/test-run/lib/tarantool-python/tests/suites/box.lua 1 warning

/home/alex/projects/tarantool-meta/r/t-3/test-run/lib/tarantool-python/test-run/lib/tarantool-python/test-run/lib/tarantool-python/tests/suites/box.lua:2:1: (W121) setting read-only global variable os

Checking /home/alex/projects/tarantool-meta/r/t-3/test-run/lib/tarantool-python/test-run/lib/tarantool-python/test-run/test_run.lua 2 warnings

/home/alex/projects/tarantool-meta/r/t-3/test-run/lib/tarantool-python/test-run/lib/tarantool-python/test-run/test_run.lua:2:7: (W211) unused variable json
/home/alex/projects/tarantool-meta/r/t-3/test-run/lib/tarantool-python/test-run/lib/tarantool-python/test-run/test_run.lua:161:19: (W211) unused variable info

Totktonada added a commit to tarantool/tarantool-python that referenced this pull request Feb 16, 2021
In short: this commit moves the basic MeshConnection test cases to the
`./setup.py test` test suites (which are located in the unit/ now) and
removes the test-run submodule, which is not needed anymore.

A bit background: we have a testing framework called test-run, whose
primary goal is to give ability to manage tarantool instances from a
test. Historically tarantool-python does not use test-run for testing
(it uses several simple python helpers instead), but a test-run based
test was added in the scope of #106. The idea was to reuse test-run code
more and eventually port other tests to test-run.

The objective reality reveals several problems in the idea, which looked
nice in theory.

The main problem is the cyclic dependency between test-run and
tarantool-python submodules. It consumes an extra time at recursive git
clone and places old submodule revisions in a deeply nested level. The
latter may confuse linter tools, which search for files recursively (see
[1]).

Other problems look solvable, but I'll list them, because they give
considerable weight in my impression that we should get rid of the
test-run submodule within this repository:

1. test-run based tests were not run in CI and may break silently so
   (it already occurs once).
2. The first bullet looks easy to fix, but it is unclear whether it is
   right to depend on a submodule in the `./setup.py test` testing or we
   should keep only built-in and packaged testing tools in the
   dependencies.
3. Porting tests to test-run may require extra effort and nobody was
   eager to pay time for that.
4. Existing tooling for managing tarantool instances is enough for
   testing of the connector and, at the same time, it is quite simple.
   So if we'll meet a problem, it is easier to fix.
5. test-run supports only Python 2 at the moment (however it'll be fixed
   soon, see [2]).

To sum up, the experiment with the test-run submodule looks unsuccessful
and I think we should stop it for now. If we'll decide to try again, we
should consider all described problems and implement everything in a way
that does not hurt us.

[1]: tarantool/test-run#266 (comment)
[2]: tarantool/test-run#20

Fixes #111
Totktonada added a commit to tarantool/tarantool-python that referenced this pull request Feb 18, 2021
In short: this commit moves the basic MeshConnection test cases to the
`./setup.py test` test suites (which are located in the unit/ now) and
removes the test-run submodule, which is not needed anymore.

A bit background: we have a testing framework called test-run, whose
primary goal is to give ability to manage tarantool instances from a
test. Historically tarantool-python does not use test-run for testing
(it uses several simple python helpers instead), but a test-run based
test was added in the scope of #106. The idea was to reuse test-run code
more and eventually port other tests to test-run.

The objective reality reveals several problems in the idea, which looked
nice in theory.

The main problem is the cyclic dependency between test-run and
tarantool-python submodules. It consumes an extra time at recursive git
clone and places old submodule revisions in a deeply nested level. The
latter may confuse linter tools, which search for files recursively (see
[1]).

Other problems look solvable, but I'll list them, because they give
considerable weight in my impression that we should get rid of the
test-run submodule within this repository:

1. test-run based tests were not run in CI and may break silently so
   (it already occurs once).
2. The first bullet looks easy to fix, but it is unclear whether it is
   right to depend on a submodule in the `./setup.py test` testing or we
   should keep only built-in and packaged testing tools in the
   dependencies.
3. Porting tests to test-run may require extra effort and nobody was
   eager to pay time for that.
4. Existing tooling for managing tarantool instances is enough for
   testing of the connector and, at the same time, it is quite simple.
   So if we'll meet a problem, it is easier to fix.
5. test-run supports only Python 2 at the moment (however it'll be fixed
   soon, see [2]).

To sum up, the experiment with the test-run submodule looks unsuccessful
and I think we should stop it for now. If we'll decide to try again, we
should consider all described problems and implement everything in a way
that does not hurt us.

[1]: tarantool/test-run#266 (comment)
[2]: tarantool/test-run#20

Fixes #111
Totktonada added a commit that referenced this pull request Feb 24, 2021
This change removes the test-run submodule from `lib/tarantool-python`.

The idea is to get rid of this 'recursive' nesting of test-run and
tarantool-python submodules into each other. Those nested submodules
contain old resivions of the code. It confuses linters, which looking
for code recursively (such as luacheck). See [1] for details.

After this commit we can eliminate the `test-run/**/*.lua` exclusion
from `.luacheckrc` in tarantool.

As the side effect, `git clone --recursive <...>` will be faster.

[1]: tarantool/tarantool-python#189

Follows up PR #266
Totktonada added a commit that referenced this pull request Mar 8, 2021
This change removes the test-run submodule from `lib/tarantool-python`.

The idea is to get rid of this 'recursive' nesting of test-run and
tarantool-python submodules into each other. Those nested submodules
contain old resivions of the code. It confuses linters, which looking
for code recursively (such as luacheck). See [1] for details.

After this commit we can eliminate the `test-run/**/*.lua` exclusion
from `.luacheckrc` in tarantool.

As the side effect, `git clone --recursive <...>` will be faster.

[1]: tarantool/tarantool-python#189

Follows up PR #266
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants