Allow DrupalExtension::login() (and others) to be overridden more cleanly

Here's a first pass at making things more generic.

It changes the login() method to userLogin() and then adds a new login() method that can be overridden. It adds language to distinguish between @Given and @Then when checking for if a user is logged in or not. And it adds an alternative login method for specifying the user/password for a role from the yml file, so that you can write tests that don't create new users, but login with existing users with the right roles.

Here's an updated patch, not much different from the last one, but a more consistent language for Given/When verses Then.

See http://docs.behat.org/guides/1.gherkin.html#steps.

"Behat doesn’t technically distinguish between these three kind of steps. However, we strongly recommend that you do! These words have been carefully selected for their purpose, and you should know what the purpose is to get into the BDD mindset."

We, as test writers, need to use consistent language to help with this distinction. So I'm trying to follow the guideline that "When" and "Given" steps have an action verb and most "Then" steps start with "I should".

* When/Given can do things
* Then should just check things

Also, the step hooks themselves should honor this distinction. Thus, the current code does the same thing for iVisit and iAmAt, and this IMO, is wrong. iVisit is an action When/Given, but iAmAt is a Then conditional check. While I didn't fix the iVisit/iAmAt in this issue, since this issue is about login, I did fix the same problem as it related to logging in and out.

Catch pointed me to http://loginisnotaverb.com for why not use "login," and I pointed him to http://dictionary.reference.com/browse/login which actually shows it as a verb. At the very least, I'd suggest the "login" or "log in" is still a developing word in our vocabulary, and "login" may still be slang, but ... it is widely used by the Technical community, and to make test writing easier, we should support it.

For anything that uses a hard-coded CSS selector, I'd prefer to have that go through something similar to the hard-coded text ('Log out', etc) so it is easier to override without having to override the entire method.

As for the new whoami() method (in doobie, this method predates the ability to directly create users via drush), is there any reason we can't use the class variable $user as was previously done to track the current user?

Sure, a selector as a first level way of overriding things is fine with me. But I don't see that working in many cases. What if the site doesn't want to put the information on the page at all, but instead, maybe puts it in a SESSION variable. IMO, you need the override, you don't need the setting, but there's no harm to having both.

As for the whoami, the new code does not assume that you create new users for every test. You can login as iLoginAsUser() or iLoginAsANewUser(). But the difference between $this->user and whoami() is that $this->user is the user you want to login as, and whoami() is the user the site says you are logged in as. From a testing framework perspective, shouldn't we always get this from the site?

This patch works well for me. I tested overriding whoami() to return a value from the page.

• The current whoami() forces the page to redirect to /user just to read the page title, "Given" Gherkin statements shouldn't change the website state, and we are changing the current page, which is a big change in state.
• If we want to use these tests as part of performance tests (and we do), the tests shouldn't do too much extra work outside the normal workflow of clicking around the site, and as this works currently, a redirect to the /user page is quite a bit of extra work. (This same rationale is one reason why I don't like to create or tear down users.)

I did not add the requested change to make the selector configurable however, because all the other configurable values are retrieved with getDrupalText() because they are text strings, yet this is no longer a text string but instead a selector, and I'm not sure how you'd like to store those. Also, FWIW, IMO we need to start providing defaults for these configurable items so that the test writer isn't forced to set dozens of configurables, but only the ones they want to override.

Also, FWIW, IMO we need to start providing defaults for these configurable items so that the test writer isn't forced to set dozens of configurables, but only the ones they want to override.

The latest version should provide defaults--it was never intended that test writers implement all of the defaults :)

I've overlooked a lot of non-standard usage in the steps and comments of the Drupal.org BDD project while learning and trying to keep up with the pace of work there, but I do favor being deliberate, especially with the DrupalExtension.

In this specific case, I'd say a phrasal verb like "log in" is unlikely to be formally accepted as single word any time soon. "She logs in" isn't likely to become "She logins" Same with past tense: "I logged in yesterday" is preferable to "I loginned yesterday."

I don't disagree that many people write login in the first person present and imperative, etc. They also leave trailing white space in their files and don't indent properly.

Drupal.org uses the Chicago Manual of Style as the comprehensive reference, and Drupal.org's industry word list at http://drupal.org/node/338208 specifies "log in" for the verb form. I'd go with that. As much as I love that language changes and evolves, I don't relish the idea of this project being on the bleeding edge :)

This does get me thinking some tangentially-related things:

We need to balance both the ease of writing tests for tech people and the ease of reading tests for stakeholders and really, anyone else who didn't write the tests.

Noun forms evolve much more quickly than the grammar rules for conjugating verbs, and style guides surely do lag behind and contradict each other. Not all clients will use the Chicago Manual, and we are going to need to support e-mail and email, website and web site, etc.

I'd like to split up some of this patch to attempt to achieve the following:

• Push the login method back to specific drivers, allowing the Drush driver to log in with drush uli, and the Drupal driver could do whatever makes sense (I'm not sure offhand how possible programmatic login is via the api). There is this issue already for drush #1846828: Use drush user-login to login user..
• As far as easily overriding, or hooking into that process, it may make sense to use the annotations that Behat already uses so heavily, perhaps tagging methods as userLogin or some such. Related to this would be an easy way to do that for node creation as well. #1823602: Need a framework for creating nodes (in drush driver)

I can't quite tell - was this implemented?

I have a login process that needs to accept "Terms of Use" before the user is logged in. I've pulled login() into my FeatureContext and am adding:

$element->checkField('edit-termsofuse-accept');
$element->pressButton('edit-submit');

The code isn't working yet (still debugging, which I find painful also) but would be nice if I could simply override the existing login() rather than completely rewrite/shadow it.

I would prefer to implement this in the same way nodes, users and terms are currently extensible. For instance, before a node is created, the beforeNodeCreate event is dispatched. Similarly, we could define and dispatch beforeLogin() and similar after initial fields are filled in, but prior to hitting submit.

I'm hoping this is as good a place as any to report this. I'm trying to override DrupalContext::afterScenario()

I have added a step definition to login as user 1 because our site doesn't have a global administrator role. However it adds user 1 to the private $users array. My override of afterScenario() has no access to $this->users though. I excluded the uid for user 1 so it no longer tries to delete but what is the particular reason it's setup as a private variable?