-
Notifications
You must be signed in to change notification settings - Fork 46
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
Run all tests under CI #111
Labels
Comments
Totktonada
changed the title
Enable unit & functional testing in CI
Run test-run tests under CI
Mar 14, 2019
test-run tests are not run with
|
Totktonada
added
3sp
backlog
code health
Improve code readability, simplify maintenance and so on
prio4
labels
Dec 21, 2020
This was referenced Feb 10, 2021
Merged
Totktonada
added a commit
that referenced
this issue
Feb 16, 2021
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
Totktonada
added a commit
that referenced
this issue
Feb 16, 2021
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
Totktonada
added a commit
that referenced
this issue
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
that referenced
this issue
Feb 18, 2021
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
Totktonada
added a commit
that referenced
this issue
Feb 18, 2021
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
Totktonada
added a commit
that referenced
this issue
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
It was blocked by an old tarantool-python submodule in test-run (tarantool/test-run#126) as @kbelyavs says, but now it does not.
The text was updated successfully, but these errors were encountered: