bpo-37596: Update `test_deterministic_sets` to correctly handle different string hash algorithms by brandtbucher · Pull Request #28147 · python/cpython · GitHub
Skip to content

bpo-37596: Update test_deterministic_sets to correctly handle different string hash algorithms - #28147

Merged
brandtbucher merged 1 commit into
python:mainfrom
brandtbucher:fix-marshal-tests
Sep 4, 2021
Merged

bpo-37596: Update test_deterministic_sets to correctly handle different string hash algorithms#28147
brandtbucher merged 1 commit into
python:mainfrom
brandtbucher:fix-marshal-tests

Conversation

@brandtbucher

@brandtbucher brandtbucher commented Sep 3, 2021 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Member 
$ for HASH_ALGORITHM in siphash24 fnv
> do
>     ./configure --with-hash-algorithm=$HASH_ALGORITHM &> /dev/null
>     make -j &> /dev/null
>     ./python -c 'import sys; print(f"{sys.hash_info.algorithm = }")'
>     ./python -m test test_marshal
> done
sys.hash_info.algorithm = 'siphash24'
0:00:00 load avg: 3.28 Run tests sequentially
0:00:00 load avg: 3.28 [1/1] test_marshal

== Tests result: SUCCESS ==

1 test OK.

Total duration: 316 ms
Tests result: SUCCESS

sys.hash_info.algorithm = 'fnv'
0:00:00 load avg: 15.70 Run tests sequentially
0:00:00 load avg: 15.70 [1/1] test_marshal

== Tests result: SUCCESS ==

1 test OK.

Total duration: 311 ms
Tests result: SUCCESS

https://bugs.python.org/issue37596

@brandtbucher brandtbucher added tests Tests in the Lib/test dir skip news labels Sep 3, 2021
@brandtbucher
brandtbucher requested a review from tiran as a code owner September 3, 2021 19:02
@brandtbucher brandtbucher added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Sep 3, 2021
@bedevere-bot

bedevere-bot commented Sep 3, 2021

Copy link
Copy Markdown

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Sep 3, 2021
gvanrossum
gvanrossum approved these changes Sep 3, 2021
View reviewed changes

@gvanrossum gvanrossum left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So did you just try different strings until you found one that hashes the same under fnv and siphash24?? That's amazing. (I didn't even know we had all this machinery -- all I know is that we have "hash randomization" and that you can force the seed by setting an env var. :-)

@brandtbucher

brandtbucher commented Sep 3, 2021

Copy link
Copy Markdown
Member Author

So did you just try different strings until you found one that hashes the same under fnv and siphash24?? That's amazing.

No, I just needed to find one that hashes in such a way that sets containing them have different orders under PYTHONHASHSEED=0 and PYTHONHASHSEED=1 for each algorithm! It's a much easier task. 🙃

(I didn't even know we had all this machinery -- all I know is that we have "hash randomization" and that you can force the seed by setting an env var. :-)

Same, I learned a lot about this stuff today!

@brandtbucher

brandtbucher commented Sep 4, 2021

Copy link
Copy Markdown
Member Author

That buildbot's been failing for a while with that same error. I'll open an issue for it.

@brandtbucher
brandtbucher merged commit 3beef86 into python:main Sep 4, 2021
@bedevere-bot

bedevere-bot commented Sep 4, 2021

Copy link
Copy Markdown

@brandtbucher: Please replace # with GH- in the commit message next time. Thanks!

@bedevere-bot

bedevere-bot commented Sep 4, 2021

Copy link
Copy Markdown

@brandtbucher
brandtbucher deleted the fix-marshal-tests branch July 21, 2022 20:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gvanrossum gvanrossum gvanrossum approved these changes
@tiran tiran Awaiting requested review from tiran

Assignees

No one assigned

Labels

skip news tests Tests in the Lib/test dir

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@brandtbucher @bedevere-bot @gvanrossum @the-knights-who-say-ni