Make 'nailgun.entity_fields.URLField.gen_value' method always generates unique value #190
Conversation
| names must be unique. | ||
| By default, :meth:`nailgun.entity_fields.StringField.gen_value` can | ||
| produce strings in both lower and upper cases, but domain name should | ||
| be always in lower case due logical reason. |
There was a problem hiding this comment.
Thanks for fixing this. I must've copied and pasted. Blech!
|
ACK. Fixes #189. |
|
ACK |
Make 'nailgun.entity_fields.URLField.gen_value' method always generates unique value
| @@ -265,4 +266,4 @@ class URLField(StringField): | |||
|
|
|||
| def gen_value(self): | |||
| """Return a value suitable for a :class:`URLField`.""" | |||
| return gen_url() | |||
| return gen_url(subdomain=gen_alpha()) | |||
There was a problem hiding this comment.
9gag and 4chan fans might get upset about this solution..how about this?
return gen_url(subdomain=gen_alphanumeric())also, how about including a . to support multiple-domain format (like foo.bar.org)?
of course it must not be at the beginning or at the end of the string
There was a problem hiding this comment.
We do not restrict or introduce any limitation to possible URL, so if you like to use particular URL in your specific test case - just do it, but for generic scenario (for create_missing() method) I prefer to use most basic generator method, so it will be applicable for most possible situations
There was a problem hiding this comment.
@rplevka You're right. It'd be good to generate more "interesting" data. I didn't both pushing for the change, though, because this PR really just works around issues in the underlying fauxfactory library. An even better solution is to make the gen_url function produce a greater variety of URLs and obey RFC 2606.
There was a problem hiding this comment.
[...] this PR really just works around issues in the underlying fauxfactory library.
What are they? Is there an issue filed to track them?
There was a problem hiding this comment.
@omaciel There's two issues:
gen_url,gen_emailand possibly others generate real values that might be in use in the wild. For example,gen_urlmight spit out test.com.gen_urldoesn't produce especially random values. That's why we're callinggen_url(subdomain=gen_alpha())instead ofgen_url()here in NailGun.
omaciel/fauxfactory#26 is describes how gen_email produces "real" values. There aren't issues for the other methods that produce "real" values. There's not an issue for gen_url's lack of randomness.
There was a problem hiding this comment.
@omaciel from my observation gen_url method give us not so many variations of inputs and such data become similar pretty fast. Let's say, it generate:
test.com, example.com, test.biz, example.biz and few more, so even for 10 tests there is chance that we will have duplicated entry. Using proposed approach we can decrease such chances in hundreds times
|
👍 |
+Removing corresponding workaround
+Removing redundant unit tests