test: get rid of the test-run submodule #189
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
It was initially supported (since 0.6.5 release, [1]), but in 0.6.6 ([2]) we tighten arguments validation with only Internet sockets in the mind. This breaks test/cluster-py/multi.test.py test, which uses Unix sockets. Now it works again. [1]: 495dec1 ('Implement reconnection strategy class') [2]: 4dfe5f9 ('Support cluster discovery in MeshConnection') Part of #111
The scenario is the following: 1. Create and start a server (TarantoolServer instance). 2. Stop it. 3. Call `<server>.admin(<command>)`. I intend to follow this scenario in a test case in the following commit. We should close old socket at stopping the server, because an attempt to write to the old socket may not fail even if the other end is already gone. If we'll close the console connection (TarantoolAdmin instance) explicitly, it'll be reconnected at the next usage. Part of #111
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
This virtually reverts commit 436218d ('move unit tests to unit'), that was made in the scope of #106. Despite the fact that testing of the connector uses `unittest` framework, it is functional (and integration) testing by its nature: most of the test cases verify that public API of the connector properly works with tarantool. In seems meaningful to locate such kind of test cases in the `test/` directory, not `unit/`, disregarding of used framework. Follows up #106.
Totktonada
deleted the
Totktonada/gh-111-get-rid-of-test-run-submodule
branch
Totktonada
added a commit
to tarantool/test-run
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
to tarantool/test-run
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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes in short:
./setup.py testtest suites.unit/totest/.See commit messages for more in depth explanation.
Fixes #111.