Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dawehner’s picture

Status: Active » Needs review
FileSize
5.95 KB

#1998466: Convert filter_xss_admin and similar function to an Xss component is the only global dependency which blocks from convering to unit tests.

Status: Needs review » Needs work

The last submitted patch, drupal-2003934-1.patch, failed testing.

dawehner’s picture

Status: Needs work » Needs review
FileSize
658 bytes
5.92 KB
dawehner’s picture

#3: drupal-2003934-2.patch queued for re-testing.

dawehner’s picture

FileSize
6.05 KB

Just a rerole.

Status: Needs review » Needs work

The last submitted patch, drupal-2003934-5.patch, failed testing.

dawehner’s picture

Status: Needs work » Needs review

#5: drupal-2003934-5.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, drupal-2003934-5.patch, failed testing.

dawehner’s picture

Status: Needs work » Needs review
FileSize
812 bytes
6.09 KB

Let's fix the failures.

ParisLiakos’s picture

Issue tags: +PHPUnit Blocker
+++ b/core/core.services.ymlundefined
@@ -427,3 +427,6 @@ services:
+  format_date:
+    class: Drupal\Core\Datetime\DateFormats

why not name the service date? then we can add more helper methods in the future

+++ b/core/lib/Drupal/Core/Datetime/DateFormats.phpundefined
@@ -0,0 +1,112 @@
+      $langcode = language(Language::TYPE_INTERFACE)->langcode;

we can inject language manager here and use instead

Status: Needs review » Needs work
Issue tags: -PHPUnit Blocker

The last submitted patch, drupal-2003934-9.patch, failed testing.

tim.plunkett’s picture

Status: Needs work » Needs review
Issue tags: +PHPUnit Blocker

#9: drupal-2003934-9.patch queued for re-testing.

dawehner’s picture

FileSize
1005 bytes
6.13 KB

Yeah why not.

Status: Needs review » Needs work

The last submitted patch, drupal-2003934-13.patch, failed testing.

damiankloip’s picture

Status: Needs work » Needs review
FileSize
1.96 KB
6.6 KB

Rerolled and injected the language manager.

Status: Needs review » Needs work
Issue tags: -PHPUnit Blocker

The last submitted patch, 2003934-15.patch, failed testing.

tim.plunkett’s picture

Status: Needs work » Needs review

#15: 2003934-15.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +PHPUnit Blocker

The last submitted patch, 2003934-15.patch, failed testing.

dawehner’s picture

Status: Needs work » Needs review
FileSize
1.01 KB
6.59 KB

Let's use a central config variable.

ParisLiakos’s picture

ok sorry for this, but i just saw DrupalDateTime object..and i am sitting here and wondering why this logic shouldnt be in DrupalDateTime::format since it is the drupal specific object?

so i can do:

$date = new DrupalDateTime(REQUEST_TIME);
$date->format('short');

or even introduce a create() method to make this oneliner...

tim.plunkett’s picture

No, we don't want to do that because of #2003892: Convert date formats to config entities

Status: Needs review » Needs work

The last submitted patch, drupal-2003934-19.patch, failed testing.

damiankloip’s picture

Status: Needs work » Needs review
FileSize
616 bytes
6.58 KB

We have a mismatch of service names.

Status: Needs review » Needs work

The last submitted patch, 2003934-23.patch, failed testing.

damiankloip’s picture

Status: Needs work » Needs review
FileSize
609 bytes
6.59 KB

Sorry, totally put the wrong constant in the language change :)

That nicely vouches for the test coverage I guess.

dawehner’s picture

Component: views.module » base system

Nice! Not everything is views btw. :)

ParisLiakos’s picture

Status: Needs review » Reviewed & tested by the community

looks great to me:) thanks!

alexpott’s picture

Needs reroll

curl https://drupal.org/files/2003934-25.patch | git a
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  6746  100  6746    0     0   6359      0  0:00:01  0:00:01 --:--:--  7588
error: patch failed: core/core.services.yml:440
error: core/core.services.yml: patch does not apply
alexpott’s picture

Status: Reviewed & tested by the community » Needs work
tim.plunkett’s picture

Status: Needs work » Reviewed & tested by the community
FileSize
6.58 KB

This patch and #2004506: Move standard_country_list() to Drupal\Core\Locale\Country both tried to add a service to the end of the core.services.yml.
Since it was just a failing of git and absolutely nothing changed, I'm reRTBCing.

Please don't credit me in the commit message.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 0bfe623 and pushed to 8.x. Thanks!

Automatically closed -- issue fixed for 2 weeks with no activity.