Contrib.social

drupalLogin() crashes where it should fail.

Open on Drupal.org →
Created on 5 April 2014, over 10 years ago
Updated 25 July 2024, 4 months ago

Problem/Motivation

I should be able to write a test that fails without crashing.

There are a number of SimpleTest assertions that crash rather than fail.

One of them is drupalLogin().

Typically, you do something like this:

$account = $this->drupalCreateUser(array('my special permission'));
$this->drupalLogin($account);

I should be able to write that test, run it, and have it fail without a code error, even before I implement hook_permission().

drupalCreateUser() does the right thing by returning FALSE if my permission doesn't exist.

drupalLogin() does the WRONG thing by using type hinting instead of checking its input. drupalLogin() should give me a fail if I don't have a valid user, rather than a code error.

Proposed resolution

Remove type hinting from drupalLogin(), and have it show a fail if the account object is invalid. Also, short-circuit the remaining login process.

Remaining tasks

User interface changes

API changes

🐛 Bug report
Status

Postponed: needs info

Version

11.0 🔥

Component
PHPUnit  →

Last updated 18 minutes ago

Created by

🇺🇸United States mile23 Seattle, WA

Live updates comments and jobs are added and updated live.
  • Needs tests

    The change is currently missing an automated test that fails when run with the original code, and succeeds when the bug has been fixed.

  • Needs issue summary update

    Issue summaries save everyone time if they are kept up-to-date. See Update issue summary task instructions.

Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • Comment over 1 year ago →
    🇺🇸United States smustgrave

    This came up as a daily bugsmash target. Looking at #5 since simpletest has been removed is this still an issue?

    When simpletest was removed https://www.drupal.org/node/3091784 →

  • Status changed to Needs work 5 months ago
  • Comment 5 months ago →
    🇦🇺Australia acbramley

    Checked the methods and yes this still applies to phpunit, as per #32 and #5 we should throw an exception in createRole instead of returning FALSE.

    We should start with a reroll of #5 on an MR against 11.x

  • First commit to issue fork.
  • Merge request !8919Issue #2234295: drupalLogin() crashes where it should fail → (Open) created by magaki
  • Pipeline finished with Success
    4 months ago
    Total: 565s
    #233595
  • Status changed to Needs review 4 months ago
  • Comment 4 months ago →
    🇯🇵Japan magaki

    I created MR that reroll #5 to UserCreationTrait.

  • Status changed to Postponed: needs info 4 months ago
  • Comment 4 months ago →
    🇦🇺Australia acbramley

    Now that I'm actually manually testing the scenario described in the IS I don't think this is a valid bug anymore (see my comment on the MR).

    Perhaps at the time this issue was created, we didn't call $this->fail() inside checkPermissions OR as per #32 phpunit halts execution on a fail.

    Since we fail when there's an invalid permission, the original issue in the IS does not happen because drupalLogin is never called when an invalid permission is passed through.

    Settintg to PMNMI to see if there's other scenarios that we need to account for.

  • Pipeline finished with Success
    4 months ago
    Total: 445s
    #234297
contrib.social Blog FAQ Discussions
Production build 0.71.5 2024