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

tests: Removing intg/tests/test_memory_cache.py #7577

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

Conversation

danlavu
Copy link

@danlavu danlavu commented Sep 9, 2024

These tests are covered system/test_memcache.py

@alexey-tikhonov
Copy link
Member

CI is red

These tests are covered system/test_memcache.py
@danlavu
Copy link
Author

danlavu commented Sep 18, 2024

@alexey-tikhonov green now, forgot to edit the Makefile

@alexey-tikhonov
Copy link
Member

test_colliding_hashes() doesn't seem to be ported.
That's a question, though, if we want to keep it...

@danlavu
Copy link
Author

danlavu commented Sep 25, 2024

@alexey-tikhonov I vote to drop it.

@alexey-tikhonov
Copy link
Member

This test covered a bug #4595 fixed by Sumit.
@sumit-bose, are you find to drop this test or would you prefer to keep it?

@sumit-bose
Copy link
Contributor

This test covered a bug #4595 fixed by Sumit. @sumit-bose, are you find to drop this test or would you prefer to keep it?

Hi,

independent of the issue, since collisions might happen I think having a test which makes sure we can handle them, would be good.

bye,
Sumit

@danlavu danlavu marked this pull request as draft October 4, 2024 03:16
@danlavu
Copy link
Author

danlavu commented Oct 4, 2024

I will write the 'colliding hashes tests' before we drop this file, putting the PR back to draft

@danlavu danlavu marked this pull request as ready for review November 5, 2024 03:47
@danlavu danlavu marked this pull request as draft November 6, 2024 18:48
@danlavu
Copy link
Author

danlavu commented Nov 6, 2024

Sorry confused this with another PR, this isn't ready yet, back to draft.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants