pschanely / CrossHair

An analysis tool for Python that blurs the line between testing and type systems.
Other
996 stars 47 forks source link

`PurePath(LazyIntSymbolicStr)` error #280

Closed tybug closed 1 month ago

tybug commented 1 month ago

https://crosshair-web.org/?crosshair=0.1&python=3.8&gist=35e9e3726667c18ebffe94d2e88007de, in triaging test_can_generate_from_all_registered_types[PathLike] from https://github.com/HypothesisWorks/hypothesis/pull/4034.

This one is my largest reach yet in correctly setting up the prereqs. This doesn't happen for "normal" symbolic strings, only LazyIntSymbolicStr. If I was to backseat and potentially embarrass myself, I would guess LazyIntSymbolicStr needs to declare _pytype as str?

Zac-HD commented 1 month ago

If I was to backseat and potentially embarrass myself, I would guess LazyIntSymbolicStr needs to declare _pytype as str?

Hmm, I see a lot of failures that this could explain actually 🤔

pschanely commented 1 month ago

Oh wow this makes me so happy. And at the risk of embarrassing myself, I'll give you a needlessly detailed response:

tybug commented 1 month ago

I really appreciate you taking the time to write this up <3. The c conversion makes sense as a critical juncture.

I admit my diagnosis of 'some symbolic strings work fine' was a conclusion based on the following passing:

from pathlib import PurePath

def f(s: str) -> None:
    '''
    post: True
    '''
    PurePath(s)
pschanely commented 1 month ago

I admit my diagnosis of 'some symbolic strings work fine' was a conclusion based on the following passing:

Oooooh! Ok, so I'd totally forgotten about this, and it's really great that you brought it up. There is a hacky part of regular CrossHair that checks TypeErrors on their way out and looks for a few CrossHair type names in the exception message. If it finds a CrossHair type name, it suppresses the error and ignores that path. This code is tucked away in a corner that I simply hadn't thought to include in the hypothesis plugin.

Part of me doesn't want to copy that logic over to the plugin because it's been so great for issue discovery. However, it probably needs to stick around because we'll never support arbitrary 3rd party C modules. Plus, it'll fix a lot of hypothesis tests, and I can always disable the feature locally to go bug hunting at my leisure. So, I think I'll proceed with propagating my hack, unless someone has a counterpoint. Filed https://github.com/pschanely/hypothesis-crosshair/issues/19

Zac-HD commented 1 month ago

Hypothesis' InvalidArgument subclasses TypeError, so you might want to use an exact-match rather than isinstance - but otherwise that plan sounds good to me 🙂

pschanely commented 1 month ago

And ... support for os.fspath on symbolic strings has landed in v0.0.64.