Skip to content

Remove luatest and checks submodules #454

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

locker
Copy link
Member

@locker locker commented Apr 11, 2025

The test-run project is rarely updated these days while luatest is rapidly developing. Let's remove the luatest submodule from test-run to ease luatest bumps in projects using test-run. Now, luatest is supposed to be added as a submodule to all projects using test-run.

By default, test-run looks for luatest in ../third_party/luatest, but this can be overridden with the new --luatestdir option.

Note that we also remove the checks submodule, which was added as a dependency of luatest. It was built in Tarantool in 2.11.0 so there's no need in it anymore.

Closes #453

The `test-run` project is rarely updated these days while `luatest` is
rapidly developing. Let's remove the `luatest` submodule from `test-run`
to ease luatest bumps in projects using `test-run`. Now, `luatest` is
supposed to be added as a submodule to all projects using `test-run`.

By default, `test-run` looks for `luatest` in `../third_party/luatest`,
but this can be overridden with the new `--luatestdir option`.

Note that we also remove the `checks` submodule, which was added as
a dependency of `luatest`. It was built in Tarantool in 2.11.0 so
there's no need in it anymore.

Closes #453
@locker
Copy link
Member Author

locker commented Apr 11, 2025

We can't remove the submodules because they are needed for integration tests. I think I'll leave them be and instead allow to specify a custom luatest directory.

@locker locker closed this Apr 11, 2025
@locker locker deleted the locker/rm-luatest-submodule branch April 11, 2025 17:02
@Totktonada
Copy link
Member

We still can do it, just add luatest installation into the CI script.

parser.add_argument(
"--luatestdir",
dest="luatestdir",
default="../third_party/luatest",
Copy link
Member

Choose a reason for hiding this comment

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

Can we search it using LUA_PATH by default?

I mean, we don't need the bin/luatest wrapper (it just makes things more complicated for us in the case), we can just do tarantool -e 'require('luatest.cli_entrypoint')()'.

Copy link
Member

Choose a reason for hiding this comment

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

I think, we don't ever need this --luatestdir argument or env variable. Just correctly configured lua paths.

Copy link
Member Author

Choose a reason for hiding this comment

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

The thing is I don't want to setup LUA_PATH when running tests in Tarantool with test-run.py. I just want everything to work by default, without any extra parameters or environment variables. Moreover, I'd like test-run.py --env to setup LUA_PATH so that I can run tests with luatest.

Copy link
Member Author

Choose a reason for hiding this comment

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

In other words, yes, we could require developers to setup luatest by themselves for running Tarantool tests, but (a) this would be inconvenient and (b) could result in non-obvious errors caused by using an old version of luatest. Better checkout luatest as a submodule in Tarantool and make test-run use it automatically IMO.

@locker locker restored the locker/rm-luatest-submodule branch April 11, 2025 17:51
@locker
Copy link
Member Author

locker commented Apr 11, 2025

We still can do it, just add luatest installation into the CI script.

Yep, that'd probably be better than leaving the luatest submodule in test-run just for the sake of those tests. However, to achieve that I'd have to teach test-run to work with luatest installed from rocks, which seems tricky. I'll try that.

TBH I find these tests useless because we check that test-run works by updating the submodule in the Tarantool project, which is much more extensive and reliable. Maintaining these tests is a burden. Actually, they don't even work with Tarantool 3.0. Given that test-run is a legacy, which should eventually be replaced with pure luatest or ctest, I'd prefer to simply drop those tests. BTW these tests were added in 2021 (76cefbe), when test-run in fact stopped developing.

@locker locker reopened this Apr 11, 2025
@locker locker self-assigned this Apr 11, 2025
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.

Remove luatest submodule
2 participants