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.

Sign up for GitHub

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

Jump to bottom

Enhance Should Contains for bytes #5054

Open
WisniewskiP opened this issue Feb 14, 2024 · 3 comments · May be fixed by #5097
Open

Enhance Should Contains for bytes #5054

WisniewskiP opened this issue Feb 14, 2024 · 3 comments · May be fixed by #5097
Labels
effort: small enhancement good first issue Good for newcomers priority: medium
Milestone
v7.1

Comments

@WisniewskiP
Copy link

To use Should Contains with bytes, item that is searched needs to be first converted to bytes.

Test
    ${binaryVar} =    Convert To Bytes    \x00\x01\x02\xA0\xB0
    Should Contain    ${binary1Var}    \xA0

result:

TypeError: a bytes-like object is required, not 'str'

for int the value is converted automatically:

Test Should Contain int
    ${binary1Var} =    Convert To Bytes    \x00\x01\x02\xA0\xB0
    ${item} =    Convert To Bytes    \xC0

    Should Contain    ${binary1Var}    ${1000}

this fails with:

ValueError: byte must be in range(0, 256)

this is expected.
@WisniewskiP WisniewskiP mentioned this issue Feb 14, 2024
Closed
@WisniewskiP WisniewskiP changed the title Enhence Should Contains for bytes Enhance Should Contains for bytes Feb 14, 2024
@pekkaklarck pekkaklarck added this to the v7.1 milestone Mar 26, 2024
@pekkaklarck
Copy link
Member

We could easily enhance Should Contain so that if the container (i.e. the first argument) is bytes and the item to check (i.e. the second argument) is a string, we convert the latter to bytes. In practice it would just require something like below (plus tests and docs):

if isinstance(container, bytes) and isinstance(item, str):
    try:
        item = item.encode('latin-1')
    except UnicodeEncodeError:
        raise ValueError(...)

@droeland
Copy link

Can I take this one up?

@droeland droeland linked a pull request Mar 29, 2024 that will close this issue
Open
@pekkaklarck
Copy link
Member

Thanks for the PR @droeland. It looks pretty good, but there are some smallish things that could be still enhanced. See the PR review for more details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
effort: small enhancement good first issue Good for newcomers priority: medium
Projects
None yet
Milestone
v7.1
Development

Successfully merging a pull request may close this issue.

Fixes #5054: Enhance Should Contains for bytes droeland/robotframework
3 participants
@pekkaklarck @WisniewskiP @droeland