Skip to content
Snippets Groups Projects

Move work from #3405696

Closed spokje requested to merge issue/drupal-3407834:3407834-update-behat-from into 11.x
4 unresolved threads

Closes #3407834

Merge request reports

Approval is optional
Code Quality is loading
Test summary results are being parsed

Closed by Alex PottAlex Pott 1 year ago (Feb 13, 2024 11:09am UTC)

Merge details

  • The changes were not merged into 11.x.

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • spokje
  • spokje added 8 commits

    added 8 commits

    Compare with previous version

  • Dave Long
  • spokje resolved all threads

    resolved all threads

  • mondrake @mondrake started a thread on the diff
  • 66 66 return preg_replace('/^\/[^\.\/]+\.php\//', '/', $path) . $query . $fragment;
    67 67 }
    68 68
    69 /**
    70 * {@inheritdoc}
    71 */
    72 public function responseHeaderEquals(string $name, $value) {
    73 if (is_null($value)) {
    74 $this->responseHeaderDoesNotExist($name);
    75 return;
    76 }
    77 parent::responseHeaderEquals($name, $value);
    78 }
    79
    • Comment on lines +69 to +79

      There are just a few instances of this method being called with $value === NULL. Would it not make sense to just convert those instances to use ::responseHeaderDoesNotExist() rather than overrding like this? It would also improve readibility IMHO.

    • Core is easily(?) fixable, but what about contrib? I believe tests with $value === NULL will also break over there?

      Doing the override would give us the chance to deprecate and not break this until 11.0.0.

      Normally hate BC stuff, and don't think we should BC on behalve of external dependencies, but dropping this breakage in a core minor, and especially when it wasn't in the alpha/beta/RC seems a bit harsh?

      Edited by spokje
    • Agh yeah BC. Still I would convert core immediately to set the scene, and add a deprecation to the override right away.

    • Yeah, for some reason I believed yesterday there was some rule about not being able to add a deprecation between an RC and a full release (so 10.2.0-rc1 and 10.2.0) hence the followup issue about this, but that's weird and couldn't find anything in the deprecation policy about this.

      Let's bite this bullet now.

    • Please register or sign in to reply
  • spokje added 1 commit

    added 1 commit

    Compare with previous version

  • spokje added 1 commit

    added 1 commit

    • 57ea521a - Added deprecation for calling Drupal\Tests\WebAssert::responseHeaderEquals()...

    Compare with previous version

  • spokje added 1 commit

    added 1 commit

    • 2f9f60dd - Added deprecation for calling Drupal\Tests\WebAssert::responseHeaderEquals()...

    Compare with previous version

  • spokje added 1 commit

    added 1 commit

    Compare with previous version

  • spokje added 1 commit

    added 1 commit

    • 9686384e - Added deprecation for setting a checkbox with a non-boolean value in...

    Compare with previous version

  • spokje added 1 commit

    added 1 commit

    • 90fd51d8 - Added deprecation for setting a checkbox with a non-boolean value in...

    Compare with previous version

  • mondrake @mondrake started a thread on the diff
  • 84 84 foreach ($edit as $name => $value) {
    85 85 $field = $assert_session->fieldExists($name, $form);
    86 86
    87 // Provide support for the values '1' and '0' for checkboxes instead of
    88 // TRUE and FALSE.
    89 // @todo Get rid of supporting 1/0 by converting all tests cases using
    90 // this to boolean values.
    91 87 $field_type = $field->getAttribute('type');
    92 if ($field_type === 'checkbox') {
    88 if ($field_type === 'checkbox' && !is_bool($value)) {
    89 @trigger_error("Setting a checkbox with a non-boolean value in " . __METHOD__ . "() is deprecated in drupal:10.2.0 and will be removed in drupal:11.0.0. Checkbox '$name' was set with value '$value'. See https://www.drupal.org/node/3408184", E_USER_DEPRECATED);
    93 90 $value = (bool) $value;
    94 91 }
    • Comment on lines +88 to 91

      I think this can be done in a separate issue (there's one open already AFAICS). Just do the minimum necessary to allow the dependency bump here.

    • Well. the minimum is what we already had.

      If we want to do that, I say we do all the deprecations afterwards.

    • Please register or sign in to reply
  • 84 84 foreach ($edit as $name => $value) {
    85 85 $field = $assert_session->fieldExists($name, $form);
    86 86
    87 // Provide support for the values '1' and '0' for checkboxes instead of
    88 // TRUE and FALSE.
    89 // @todo Get rid of supporting 1/0 by converting all tests cases using
    90 // this to boolean values.
    91 87 $field_type = $field->getAttribute('type');
    92 if ($field_type === 'checkbox') {
    88 if ($field_type === 'checkbox' && !is_bool($value)) {
    89 @trigger_error("Setting a checkbox with a non-boolean value in " . __METHOD__ . "() is deprecated in drupal:10.2.0 and will be removed in drupal:11.0.0. Checkbox '$name' was set with value '$value'. See https://www.drupal.org/node/3408184", E_USER_DEPRECATED);
    • Suggested change
      89 @trigger_error("Setting a checkbox with a non-boolean value in " . __METHOD__ . "() is deprecated in drupal:10.2.0 and will be removed in drupal:11.0.0. Checkbox '$name' was set with value '$value'. See https://www.drupal.org/node/3408184", E_USER_DEPRECATED);
      89 @trigger_error("Setting a checkbox with a non-boolean value in " . __METHOD__ . "() is deprecated in drupal:10.3.0 and will be removed in drupal:11.0.0. Checkbox '$name' was set with value '$value'. See https://www.drupal.org/node/3408184", E_USER_DEPRECATED);
    • Andrey Postnikov changed this line in version 13 of the diff

      changed this line in version 13 of the diff

    • Please register or sign in to reply
  • 66 66 return preg_replace('/^\/[^\.\/]+\.php\//', '/', $path) . $query . $fragment;
    67 67 }
    68 68
    69 /**
    70 * {@inheritdoc}
    71 */
    72 public function responseHeaderEquals(string $name, $value) {
    73 if (is_null($value)) {
    74 @trigger_error('Calling ' . __METHOD__ . '() with NULL as the $value argument is deprecated in drupal:10.2.0 and will be removed in drupal:11.0.0. Use \Drupal\Tests\WebAssert::responseHeaderDoesNotExist instead. See https://www.drupal.org/node/3408184', E_USER_DEPRECATED);
    • Suggested change
      74 @trigger_error('Calling ' . __METHOD__ . '() with NULL as the $value argument is deprecated in drupal:10.2.0 and will be removed in drupal:11.0.0. Use \Drupal\Tests\WebAssert::responseHeaderDoesNotExist instead. See https://www.drupal.org/node/3408184', E_USER_DEPRECATED);
      74 @trigger_error('Calling ' . __METHOD__ . '() with NULL as the $value argument is deprecated in drupal:10.3.0 and will be removed in drupal:11.0.0. Use \Drupal\Tests\WebAssert::responseHeaderDoesNotExist instead. See https://www.drupal.org/node/3408184', E_USER_DEPRECATED);
    • Andrey Postnikov changed this line in version 13 of the diff

      changed this line in version 13 of the diff

    • Please register or sign in to reply
  • Andrey Postnikov added 137 commits

    added 137 commits

    • 90fd51d8...57ca853a - 127 commits from branch project:11.x
    • 50d98071 - Move work from #3405696
    • a78b74be - Use stark theme for test
    • 71d8d89e - Cast less
    • 45869168 - Remove DrupalSelenium2Driver::dragTo():
    • b4b610d1 - Use match
    • 7687018c - Added deprecation for calling Drupal\Tests\WebAssert::responseHeaderEquals()...
    • bdd61c89 - Added deprecation for calling Drupal\Tests\WebAssert::responseHeaderEquals()...
    • ac1b000e - Fix deprecations
    • 93ab680f - Added deprecation for setting a checkbox with a non-boolean value in...
    • 0149f028 - Added deprecation for setting a checkbox with a non-boolean value in...

    Compare with previous version

  • added 1 commit

    • c4425910 - upgrade deprecation to 10.3.0

    Compare with previous version

  • closed

  • Please register or sign in to reply
    Loading