ScreenPyHQ / screenpy

Screenplay pattern base for Python automated UI test suites.
MIT License
27 stars 3 forks source link

`is_in_bounds` parses majorant incorrectly when bounding range is floats in string form #140

Open bandophahita opened 5 months ago

bandophahita commented 5 months ago

is_in_bounds("[1.0, 4.0]") should return a majorant of 4.0 but instead returns 0.0

i1 = is_in_bounds("(1.0, 4.0)")
print(i1)

the number is within the range of 1.0 and 0.0

bandophahita commented 5 months ago

I see the problem but my regex-fu is weak. We need a regex that can find int or float.

perrygoy commented 5 months ago

The pattern is currently:

        pattern = (
            r"^(?P<lower>[\[\(]?)"
            r"(?P<minorant>\d+).*?(?P<majorant>\d+)"
            r"(?P<upper>[\]\)]?)$"
        )

It should be:

        pattern = (
            r"^(?P<lower>[\[\(]?)"
            r"(?P<minorant>\d+\.?\d*).*?(?P<majorant>\d+\.?\d*)"
            r"(?P<upper>[\]\)]?)$"
        )

This regex should match a single . if it's present, which would raise for no match if someone accidentally puts an IP address in there or something. It doesn't match a . and then expect at least one number afterwards, but it looks like Python doesn't care if they just put a period:

>>> float("4.")
4.0
bandophahita commented 5 months ago

We should also cover the case where users submit .5. i.e. `is_in_bounds("[.5, .9]")

I found [.\d]+ works and we can let the cast to float deal with the possibility of something like IP addresses.

        pattern = (
            r"^(?P<lower>[\[\(]?)"
            r"(?P<minorant>[.\d]+).*?(?P<majorant>[.\d]+)"
            r"(?P<upper>[\]\)]?)$"
        )
    def test_bad_floats(self) -> None:
        with pytest.raises(
            ValueError, match="could not convert string to float: '1.1.1.1'"
        ):
            is_in_bounds("(1.1.1.1, 4.0]")
perrygoy commented 5 months ago

Of course .\d works because . matches any character. Use the pattern in my previous comment please!