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 test coverage for _multidict_base.py #936

Merged
merged 13 commits into from
Feb 1, 2024
Merged

Conversation

a5r0n
Copy link
Contributor

@a5r0n a5r0n commented Jan 31, 2024

NotImplemented and Iterable but not Set for _viewbaseset_{and, or, sub, xor}

Partially addresses #921.

@a5r0n a5r0n requested a review from asvetlov as a code owner January 31, 2024 10:46
@a5r0n a5r0n requested a review from webknjaz as a code owner January 31, 2024 10:58
@psf-chronographer psf-chronographer bot added the bot:chronographer:provided There is a change note present in this PR label Jan 31, 2024
CHANGES/936.contrib.rst Outdated Show resolved Hide resolved
tests/test_multidict.py Outdated Show resolved Hide resolved
tests/test_multidict.py Outdated Show resolved Hide resolved
tests/test_multidict.py Outdated Show resolved Hide resolved
tests/test_multidict.py Outdated Show resolved Hide resolved
@a5r0n
Copy link
Contributor Author

a5r0n commented Feb 1, 2024

@webknjaz if I'm not wrong using and, or, -, ^ will give errors from pytest, but I will check that once again

@webknjaz
Copy link
Member

webknjaz commented Feb 1, 2024

@a5r0n thanks for starting this effort! Those operators shouldn't cause problems in pytest. However, linters (like flake8) will be unhappy about unassigned expressions. To address that, use # noqa comments with specific error codes and a comment text explaining why that linting ignore is necessary.

@webknjaz
Copy link
Member

webknjaz commented Feb 1, 2024

Oh, and the current CI failures don't seem to be related to your patch. They'll require addressing separately. I'll have to take a look myself. You'll need to rebase your PR branch at some point, once that's fixed.

P.S. Here's a recommendation for the future PRs on GitHub: https://hynek.me/articles/pull-requests-branch/.

CHANGES/936.contrib.rst Outdated Show resolved Hide resolved
CHANGES/936.contrib.rst Outdated Show resolved Hide resolved
tests/test_multidict.py Outdated Show resolved Hide resolved
CHANGES/936.contrib.rst Outdated Show resolved Hide resolved
tests/test_multidict.py Outdated Show resolved Hide resolved
tests/test_multidict.py Outdated Show resolved Hide resolved
@Jamim

This comment was marked as resolved.

tests/test_multidict.py Outdated Show resolved Hide resolved
a5r0n and others added 6 commits February 1, 2024 03:15
`NotImplemented` and `Iterable` but not `Set` for `and, or, sub, xor`
Co-authored-by: Sviatoslav Sydorenko (Святослав Сидоренко) <wk.cvs.github@sydorenko.org.ua>
Signed-off-by: a5r0n <a5r0n@users.noreply.github.com>
webknjaz and others added 3 commits February 1, 2024 03:15
Signed-off-by: a5r0n <a5r0n@users.noreply.github.com>
Signed-off-by: a5r0n <a5r0n@users.noreply.github.com>
@webknjaz
Copy link
Member

webknjaz commented Feb 1, 2024

@a5r0n I clicked "rebase" so this PR picks up the CI fixes.

CHANGES/936.contrib.rst Outdated Show resolved Hide resolved
tests/test_multidict.py Outdated Show resolved Hide resolved
tests/test_multidict.py Outdated Show resolved Hide resolved
tests/test_multidict.py Outdated Show resolved Hide resolved
a5r0n and others added 2 commits February 1, 2024 04:25
Co-authored-by: Sviatoslav Sydorenko (Святослав Сидоренко) <wk.cvs.github@sydorenko.org.ua>
Signed-off-by: a5r0n <a5r0n@users.noreply.github.com>
tests/test_multidict.py Outdated Show resolved Hide resolved
Signed-off-by: a5r0n <a5r0n@users.noreply.github.com>
@webknjaz webknjaz merged commit 75e204c into aio-libs:master Feb 1, 2024
45 of 46 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot:chronographer:provided There is a change note present in this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants