search

Gallopsled / pwntools

CTF framework and exploit development library
http://pwntools.com
Other
11.69k stars 1.67k forks source link

Change from `pop` to `keys` for reporting proper error message #2391

Closed marinelay closed 2 months ago

marinelay commented 2 months ago

Fixes #2390

peace-maker commented 2 months ago

Cool, can you add a test to avoid this regression in the future please? We use doctests, so adding another entry to the "Examples" is the way.

marinelay commented 2 months ago

Done! But, this is my first time using doctest, so I'm not sure if this is correct...

peace-maker commented 2 months ago

I'm confused if this actually runs since it'd expect it to require an # ELLIPSIS: ... comment or similar. I'll test locally.

marinelay commented 2 months ago

Oh, I saw in the documentation for doctest that ELLIPSIS option is independent of traceback (https://docs.python.org/3/library/doctest.html#what-about-exceptions). I just tested it locally in Python 3.9.18 and it worked fine without the option!

Arusekk commented 2 months ago

I think this code originally meant to use popitem.

(Note to self: This code will be gone anyway with the migration to py3 keyword-only arguments hopefully.)

peace-maker commented 2 months ago

Thank you again!