Skip to content
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

Add assert_one_of #294

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

Add assert_one_of #294

wants to merge 1 commit into from

Conversation

yngvar-antonsson
Copy link
Contributor

@yngvar-antonsson yngvar-antonsson commented Mar 24, 2023

Add assert_one_of to assert that the value is one of several alternatives.
It could be useful to write tests that support several tarantool versions.

Copy link
Member

@DifferentialOrange DifferentialOrange left a comment

Choose a reason for hiding this comment

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

I don't quite get why I was summoned to review luatest PR since I never contributed here and don't know how the infrastructure works, but the patch seems fine.

You can also think about unifying the code with assert_covers since they do similar things.

CHANGELOG.md Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
luatest/assertions.lua Outdated Show resolved Hide resolved
@Totktonada
Copy link
Member

Totktonada commented Nov 10, 2023

AFAIK we generaly follow the LuaUnit assertion naming (but just snake case variants). There are assert_table_contains and assert_not_table_contains (code).

@DifferentialOrange
Copy link
Member

DifferentialOrange commented Nov 10, 2023

AFAIK we generaly follow the LuaUnit assertion naming (but just snake case variants). There are assert_table_contains and assert_not_table_contains (code).

For me, assert_table_contains and assert_one_of have different ideas, even though their implementation is the same.

@Totktonada
Copy link
Member

BTW, the implementation is not exactly same. You're using ipairs(), so the function is applicable only for array alike tables. However, LuaUnit uses pairs() and so it makes the function applicable for tables like {foo = 'needle', bar = 'unrelated'}.

Regarding assert_one_of name semantic: if the idea is to check that the given element is present in the given table exactly one time, the implementation doesn't check it.

However, I still think that LuaUnit names are good enough and I see no reason to use different ones.

@yngvar-antonsson
Copy link
Contributor Author

As for me assertTableContains( {'a', 'b', 'c', 'd'}, 'b' ) != assert_one_of('b', {'a', 'b'}). I would leave it as is, but maybe we could think about something like assert_one_of_many or something like that.

@ylobankov ylobankov removed their request for review October 30, 2024 14:29
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.

4 participants