Closed bittner closed 9 years ago
While I do see the merit of adding something like this, I don't think this is part of the scope of what behave-django is supposed to do. behave-django integrates behave with Django, and I feel this feature does not clearly contribute to that goal.
What you could do right now is add a section in your environment file. Something like.
def before_all(context):
def get_url(url):
// implementation
context.get_url = get_url
You will then be able to use context.get_url
anywhere you wish. :grinning:
Well, that's really what I have now. The reason why I would love to see this in behave-django is, because we have a vast number of projects, and I don't want to have a copy of this code in each and every project repository.
Really, if you're strict about keeping utility functions out of the code that integrates behave with Django, then shouldn't we remove base_url
from before_scenario(), too? get_absolute_url()
, at least, would use a naming that is already known in Django, whereas base_url
is kind of non-standard, isn't it?
I don't want to be rude. I just feel that we need to make testing simpler. Much simpler. I'd love to see a future where we write friggin' concise, super-readable Python code that does all the testing.
While I was coding behave-django, I was also using a step library called behaving. This particular library had built-in step definitions that include When I visit "/some/random/page/"
which will make the browser go to the specified url. The url is automatically prefixed with context.base_url
. This is the main reason why I chose base_url
as the name for the url prefix. So you get an easy way to access the prefix, while also getting free integration with behaving
. (No side effects if you're not using it.) I just want to put my reasoning out there.
Secondly, get_absolute_url()
is meant to return an absolute url for a specific model instance. It is not used for normalizing urls with a prefix. If we did put this functionality in, I think get_url()
would probably be better as to not confuse the developers.
Third, I really don't like that it's actually longer to type.
context.get_url(path='/blog/page/1')
context.base_url + '/blog/page/1'
context.get_url(reverse_name='blog-comment-list')
context.base_url + reverse('blog-comment-list')
If you don't like the +
syntax, maybe you could do something like
'%s%s' % (context.base_url, '/blog/page/1')
I mean to be honest I don't really mind putting it in. It's just that. It's not shorter to type, it's less explicit. I really just don't see the benefit of it. I'm sorry if this offends you, but I don't see a compelling reason to include this in behave-django
. But I don't know. Maybe I'm missing something.
Just for the sake of descriptive examples:
Shorter:
context.get_url('/blog/page/1') # first parameter is "path" by default
context.base_url + '/blog/page/1'
Explicit:
context.get_url(reverse='blog-comment-list') # let's rename it for readability
context.get_url(reverse('blog-comment-list')) # neat alternative; requires an import, though!
context.get_url(reverse('blog-comment-list', 'with', 'args', and='kwargs'))
context.base_url + reverse('blog-comment-list')
Make these code samples part of behave-django's documentation and people may love the choices.
The keyword-argument reverse
is just a shortcut (saving us from importing the reverse function) that reads like the explicit use of reverse()
. Makes one line less of code and, arguably, keeps things together that belong together (optically, using parentheses). We could still keep the base_url
as an alias for get_url()
.
Could that be good enough as a reason?
What problem are we trying to solve here? What's wrong with doing context.base_url + '/blog/1/
? Or context.base_url + reverse('foo:bar')
? Do we really need this?
I hope you can see this is both short and readable, and it's not less explicit than base_url + reverse()
.
For me the compelling reason is: I want front-end developers to be able to read, modify, write the tests specified in Gherkin. Hence, every distracting (import) line I safe, every piece of code that makes them think less, every misunderstanding that I can avoid is another step in the right direction.
For what I have seen, with +
our JS gurus are happy with code such as
context.base_url + '/en/blog/categories'
... but it makes them think when reverse()
comes into play
context.base_url + reverse('blog-categories-index')
... and with all that surrounded by parentheses there is less to interpret, I feel. The inside "belongs to" the outer function call.
context.get_url(reverse('blog-categories-index'))
I acknowledge that there's a thin line between being "religious" about code beauty and "real" compelling reasons. And of course, different people have different preferences, different taste about beauty.
I love to help developers write code that people with little Python knowledge can understand. And if with this we do exactly that, the contribution is generally useful.
Would you mind if we discussed this further in a PR?
I'm really really struggling to understand why we need to wrap a function around a concatenation.
def get_url(url):
return context.base_url + url
What?
You're telling me that developers understand this
context.base_url + '/en/blog/categories'
But not
context.base_url + reverse('blog-categories-index')
?
Is this really a thing? Has this already happened? I would understand if people who aren't well versed in Django don't understand what reverse
does. But if you're at the level where you're using behave to write automated tests for your Django application, I think there's some merit in spending five minutes looking at the documentation for Django URLs.
Again, I do not think there's a problem to be solved here. I'll re-open this issue to see if people recognize this as an actual problem.
Hey, thanks for being so open to keeping the discussion alive! That's a cool move. I appreciate that a lot.
I hope we get some traffic on this repo to hear some opinions. Or maybe you simply start liking the examples in the PR! :innocent:
This was implemented by #12 . Thanks @bittner
I'm happy integrating Behave and Django was really easy with
behave-django
. (Awesome, thank you!)Also, the context.base_url is a neat shortcut. Though, I believe it's fairly common in a BDD test to reverse URLs using the reverse name of a resource, and feed the resulting absolute URL to selenium.
So, why don't we provide a
get_url()
function, or a get_absolute_url() after the example ofdjango.db.models.Model
? The function could return:context.get_url()
)context.get_url(path='/blog/page/1')
)context.get_url(reverse_name='blog-comment-list')
)An implementation could look something like this:
What do you think?