Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Automatically generate django forms #35

Closed
DRMacIver opened this issue Feb 10, 2015 · 15 comments · Fixed by #1761
Closed

Automatically generate django forms #35

DRMacIver opened this issue Feb 10, 2015 · 15 comments · Fixed by #1761
Labels
new-feature entirely novel capabilities or strategies

Comments

@DRMacIver
Copy link
Member

Django models and forms have more than enough metadata for us to automatically introspect them and figure out how to build one. We should have a hypothesis-extra package to do this.

@DRMacIver
Copy link
Member Author

Note: hypothesis-django is now a (experimental and unfinished) thing. Many models can be automatically generated. Forms are not yet supported but should be entirely doable.

@DRMacIver DRMacIver changed the title Automatically generate django models and forms Automatically generate django forms Sep 25, 2015
@DRMacIver DRMacIver added enhancement it's not broken, but we want it to be better help-wanted labels Sep 25, 2015
@Zac-HD Zac-HD added the new-feature entirely novel capabilities or strategies label Jun 10, 2017
@Zac-HD Zac-HD added good first issue and removed enhancement it's not broken, but we want it to be better help wanted labels Feb 20, 2018
@piyushmittal25
Copy link

@DRMacIver is the issue is still open and if it is please eleborate it more specifically.I want to contribute in it.

@Zac-HD
Copy link
Member

Zac-HD commented Apr 20, 2018

Yes, it's still open, but I'm worried that we might have different ideas of what will be involved - the good first issue label means we think it's suitable for someone who hasn't contributed to Hypothesis before, but I would not suggest it as your first open source contribution! (This issue is for developing a substantial new feature, which requires experience using and testing Django forms, and using Hypothesis to write tests.)

However! If you're open to working on other issues, #162 could use your help to move the emails() strategy from hypothesis.provisional to hypothesis.strategies. Comment on that issue if you'd like to give it a go, and feel free to ask questions 😄

DRMacIver added a commit that referenced this issue May 12, 2018
Automate updating of changelog and version in releasing
@Zac-HD
Copy link
Member

Zac-HD commented Aug 29, 2018

I can confirm (IIRC thanks to @vidyarani-dg) that hypothesis.extra.django.TestCase and hypothesis.extra.django.TransactionTestCase work with Django forms, even though we don't have form-related inference yet.

@thismatters
Copy link
Contributor

Hi! I've used models.py as a template and produced a functioning forms.py, I haven't built out any documentation or augmented the django toystore example, but I will happily finish that out if my approach seems sane and acceptable.
https://github.com/thismatters/hypothesis/blob/master/hypothesis-python/src/hypothesis/extra/django/forms.py

@Zac-HD
Copy link
Member

Zac-HD commented Jan 3, 2019

Awesome! You're definitely on the right track, and I'd love to get this in.

However - I'd like to get #1695 done first so we can start with the API we want to keep. Would you mind just holding off on this until next week? Once I'm home from my camping trip I'll be able to give a better review too 😄

@thismatters
Copy link
Contributor

Sure thing! I mainly wrote it because I wanted to use it, so I can keep using and polishing in my own project until the scheduled work is done.

@Zac-HD
Copy link
Member

Zac-HD commented Jan 8, 2019

OK! We now have #1743 (awaiting review), and I have time for some quick feedback on your draft 😄

  • I'd suggest using a topic branch, rather than master - it makes staying up to date or resolving conflicts with upstream much easier, and you'll probably need that!
  • Instead of the complicated form wrapper, you can generate from a dict: st.builds(form, st.fixed_dictionaries(result))
  • You can add more entries to the new Field lookup dict / slap more @register(df.SomeField) decorators on the same functions. The Field APIs are basically the same between models and forms, right? Oh, and allow df.Field in the various argument validation checks.
  • The from_model function should be pretty to copy-paste-tweak. from_form should also live in _impl.py; we don't need another module.
  • We'll need decent test suite for from_form, but "make enough models that we have one of every field type and feature" is more tedious than difficult, I hope, with the models tests to crib from.

@Zac-HD
Copy link
Member

Zac-HD commented Jan 10, 2019

#1743 has been merged, so we're ready for a PR when you are 🎉

@thismatters
Copy link
Contributor

I can probably work on it this weekend.

@thismatters
Copy link
Contributor

thismatters commented Jan 12, 2019

Regarding your second point, I'm using the form wrapper to allow for other keyword arguments to be passed when instantiating the form, and to package up results into its own keyword argument (namely data). E.g.:

form = MyFormClass(data=results, my_special_parameter=some_object)

(here results is not the strategies, but an example case of those strategies)

I'm not seeing how to do that with st.builds or st.fixed_dictionaries directly. Any hints?

This is important to support since form instances have the fields (as opposed to models where the class owns the fields), and the other keyword arguments can affect what fields are present in the instance (or how those fields are validated).

@thismatters
Copy link
Contributor

https://github.com/thismatters/hypothesis/blob/django-form-generation/hypothesis-python/src/hypothesis/extra/django/_impl.py

first draft using the new hotness. The few tests that I had written against the old draft seem to be running correctly against the new. I'll work on the internal testing for this next.

@Zac-HD
Copy link
Member

Zac-HD commented Jan 13, 2019

I'll save detailed feedback for when you open a PR - soon please! You're definitely on the right track and WIPs are welcome 😄. As to the instance/field/kwarg question:

Why not require a Form instance be passed to from_form (using internal.validation.check_type)? That way we dodge the problem of mixing strategy and non-strategy kwargs entirely - I'm worried that would mean we can't distinguish user error from just unusual usage.

This would look like from_form(MyForm(kwarg=123), some_field=text()). The problem of varying the kwargs too can be documented as "use flatmap": builds(MyForm, kwarg=integers()).flatmap(lambda form: from_form(form, some_field=text()). This is longer to write out, but cleanly separates the distinct parts of the process - and for longer or more complex versions we can suggest @st.composite instead.

@thismatters
Copy link
Contributor

I thought about receiving an unbound form instance, but that doesn't quite work either. The unbound form instance has the fields, but you still have to instantiate a new bound form instance (with the data and the original kwargs) at the end. From https://docs.djangoproject.com/en/2.1/ref/forms/api/#bound-and-unbound-forms:

If you have a bound Form instance and want to change the data somehow, or if you want to bind an unbound Form instance to some data, create another Form instance. There is no way to change data in a Form instance. Once a Form instance has been created, you should consider its data immutable, whether it has data or not.

I suppose the class could be introspected from the passed instance, but getting all of its kwargs right for instantiation of the bound form would be troublesome and unreliable. I don't see any alternative but to accept the form kwargs into from_form; would it be more palatable to assemble them into something explicitly documented? E.g.

def from_form(form, form_kwargs=None, **field_strategies):
    form_kwargs = form_kwargs or {}
    ...

I'll open the PR once I have some tests in place.

@Zac-HD
Copy link
Member

Zac-HD commented Jan 13, 2019

Oh, right. The explicit argument is better, yeah.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new-feature entirely novel capabilities or strategies
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants