search

litestar-org / polyfactory

Simple and powerful factories for mock data generation
https://polyfactory.litestar.dev/
MIT License
988 stars 78 forks source link

Bug: RecursionError With constrained 0 length lists #566

Closed tom300z closed 3 weeks ago

tom300z commented 1 month ago

Description

When constraining a list to be empty (max_length=0):

from pydantic import BaseModel, Field
from polyfactory.factories.pydantic_factory import ModelFactory
class TestModel(BaseModel):
    empty_list_field: list = Field(default=[], max_length=0)

class TestModelFactory(ModelFactory):
    __model__ = TestModel

TestModelFactory.build()

a recursion error occurs.

The problem seems to be in https://github.com/litestar-org/polyfactory/blob/67c57208de5ce993bdb2c7888864ac4e71964511/polyfactory/value_generators/constrained_collections.py#L49 where a default length of 1 is used if the randomly picked length (always 0) is falsy. @Goldziher do you remember why this default exists?

URL to code causing the issue

No response

MCVE

from pydantic import BaseModel, Field
from polyfactory.factories.pydantic_factory import ModelFactory
class TestModel(BaseModel):
    empty_list_field: list = Field(default=[], max_length=0)

class TestModelFactory(ModelFactory):
    __model__ = TestModel

TestModelFactory.build()

Steps to reproduce

1. Install polyfactory & pydantic (v2)
2. Run example code

Screenshots

No response

Logs

No response

Release Version

v2.16.2

Platform

[!NOTE]
While we are open for sponsoring on GitHub Sponsors and OpenCollective, we also utilize Polar.sh to engage in pledge-based sponsorship.

Check out all issues funded or available for funding on our Polar.sh dashboard

  • If you would like to see an issue prioritized, make a pledge towards it!
  • We receive the pledge once the issue is completed & verified
  • This, along with engagement in the community, helps us know which features are a priority to our users.

Fund with Polar

guacs commented 1 month ago

@tom300z thanks for reporting this! I checked, and I couldn't really see a good reason to have it default to 1. Some of the tests check that the length of the collection from handle_constrained_collection is greater than 0, but I think that's incorrect. It should be checking that the length is greater than or equal to 0. If I make that change, the tests are all passing as well. So I think it should be fine to not default to 1 even if the random length we get is 0.