search

fcurella / django-fakery

🏭 An easy-to-use implementation of Creation Methods for Django, backed by Faker.
http://django-fakery.readthedocs.org/en/stable/
MIT License
115 stars 6 forks source link

Make fields() update instead of overwriting #51

Closed jacobian closed 5 years ago

jacobian commented 5 years ago

This will fix #50.

This makes Blueprint.fields() return a new Blueprint, with updated fields, instead of overwriting the fields on the blueprint.

coveralls commented 5 years ago

Coverage Status

Coverage increased (+0.4%) to 92.941% when pulling 0910d09e39fa4b37a46d605a8b6ff81865dac34d on jacobian:blueprint-fields-copy into 16b0afd3a2f95212ce38c6239cf86ab27454dba5 on fcurella:master.

fcurella commented 5 years ago

@jacobian could add some tests for Bluprint.make and Blueprint.make_one as well?

jacobian commented 5 years ago

@fcurella BlueprintTest.test_blueprint calls Blueprint.make; is that good enough for the make case or would you prefer a specific test for that method? And for both, should I include something with .fields().make() / .fields().make_one() as well? I'm not sure how granular you want tests here but I don't feel strongly myself, so lmk and I'll do the needful.

fcurella commented 5 years ago

@jacobian Coveralls shows these 2 lines as missing coverage:

https://coveralls.io/builds/26358141/source?filename=django_fakery/blueprint.py#L40 https://coveralls.io/builds/26358141/source?filename=django_fakery/blueprint.py#L69

TBH all I care is that those if branches are hit at least once during the full test run

jacobian commented 5 years ago

@fcurella 👌 ah perfect, I'll do that. Thanks!

jacobian commented 5 years ago

Well... I fixed the coverage thing, but now a seemingly-unrelated test is failing. It passes locally, so I'm not sure what's up.

fcurella commented 5 years ago

@jacobian I've re-run the job and test is passing. I'll have to look into it later.

Thank you so much for your PR!