Problem/Motivation

In order to convert more tests from WTB to BTB in one big bang conversion we want to get some estimation of what kind of failures might exist.

Proposed resolution

This issue should provide a gigantic patch which does the following and JUST the following:

Instructions how to work with this patch:

  • Apply the patch
  • Look at a test failure
  • Figure out whether a) There is some missing stuff in BTB, open a follow issue for that. Exclude this particular test for now with a documentation about which issue this is blocked on b) If this test needs manual adaption, try to create issues for them as well
  • When you changed anything at the script run the following:
    rm -Rf core/modules && git checkout 8.3.x core/modules && php browsertest-convert.php && git add core/modules
    

Remaining tasks

Announcement text proposal

Title A big chunk of old tests using WebTestBase wil be converted to PHPUnit BrowserTestBase on Feb 21st
As part of the PHPUnit initiative a considerable part of Simpletests will be converted to PHPUnit based browser tests on February 21st 2017. A backwards compatibility layer has been implemented so that many Simpletests can be converted by just using the new BrowserTestBase base class and moving the test file. There is also a script to automatically convert test files in the conversion issue.

Developers are encouraged to use BrowserTestBase instead of Simpletest as of Drupal 8.3.0, but both test systems are fully supported during the Drupal 8 release cycle. The timeline for the deprecation of Simpletest's WebTestBase is under discussion.

Files: 

Comments

dawehner created an issue. See original summary.

Mile23’s picture

Status: Active » Needs review
FileSize
205.96 KB

If you say this:

grep -rl 'use Drupal\\simpletest\\WebTestBase' ./ | xargs sed -i '' 's/use Drupal\\simpletest\\WebTestBase/use Drupal\\Tests\\BrowserTestBase as WebTestBase/g'

You end up with this patch.

Which will fail.

Apocalyptically. :-)

dawehner’s picture

Status: Needs review » Needs work

The last submitted patch, 2: 2770921_2.patch, failed testing.

Mile23’s picture

Issue summary: View changes
Status: Needs work » Needs review

This time with https://www.drupal.org/files/issues/browsertest-convert.php_.txt

Already I noticed that there are some tests which use traits in the same namespace, which are missing after the move.

Mile23’s picture

And the patch. :-)

Status: Needs review » Needs work

The last submitted patch, 6: 2770921_5.patch, failed testing.

xjm’s picture

This is awesome!

xjm’s picture

Some things I noticed looking at #2 (which is actually a pretty clever way to start out here):

  1. fail: [Other] Line 121 of core/modules/filter/src/Tests/FilterFormatAccessTest.php:
    Drupal\filter\Tests\FilterFormatAccessTest::testFormatPermissions
    PHPUnit_Framework_Exception: Fatal error: Cannot use object of type Behat\Mink\Element\NodeElement as array in /var/www/html/core/modules/filter/src/Tests/FilterFormatAccessTest.php on line 153

    43 like that (I think) already in the tests that managed to report results; I wonder if a temporary BC shiv with ArrayAccess would be useful? No idea whether the object structure is remotely similar.

  2. WTB::drupalGetTestFiles() has 45 usages. I bet that could live in a trait.
  3. fail: [Other] Line 58 of core/modules/file/src/Tests/Views/RelationshipUserFileDataTest.php:
    Drupal\file\Tests\Views\RelationshipUserFileDataTest::testViewsHandlerRelationshipUserFileData
    PHPUnit_Framework_Exception: PHP Strict Standards:  Declaration of Drupal\file\Tests\Views\RelationshipUserFileDataTest::setUp() should be compatible with Drupal\views\Tests\ViewTestBase::setUp($import_test_views = true) in /var/www/html/core/modules/file/src/Tests/Views/RelationshipUserFileDataTest.php on line 17
    

    That's interesting.

Also noticed a couple things to discuss on #2770549: [plan] Discuss desired API and test suite following deprecation of SimpleTest.

dawehner’s picture

Status: Needs work » Needs review
FileSize
528.25 KB

This patch had a bit pointing to .DS_STORE

@mile23
There is an example.gitignore you might consider using :)

xjm’s picture

  1. AssertContentTrait::getRawContent() has 64 usages; might be another good candidate for a BC wrapper for BTB.
  2. WTB::assertCacheContext() has 20 calls. That does not seem like it belongs on the main class. Looking at the code, it might be doable as a shared trait. We already have BC for drupalGetHeader().
  3. drupalPost()/curlExec()/curlInitialize() look like an interesting chunk of things. (Also drupalPost() is a too-confusing name with drupalPostForm().)
  4. Update tests are a whole can of worms; on a casual scan, they might merit their own issue in a hypothetical mass "convert all the things" plan.
  5. fail: [Other] Line 67 of core/modules/field/src/Tests/EntityReference/EntityReferenceAdminTest.php:
    Drupal\field\Tests\EntityReference\EntityReferenceAdminTest::testFieldAdminHandler
    Argument 1 passed to Drupal\field\Tests\EntityReference\EntityReferenceAdminTest::getAllOptionsList() must be an instance of SimpleXMLElement, instance of Behat\Mink\Element\NodeElement given, called in /var/www/html/core/modules/field/src/Tests/EntityReference/EntityReferenceAdminTest.php on line 508 and defined
    

    Similar to #9.1.

  6. The xpath content assertions are a big chunk for a BC layer.

Status: Needs review » Needs work

The last submitted patch, 10: 2770921-9.patch, failed testing.

xjm’s picture

Mile23’s picture

(Also drupalPost() is a too-confusing name with drupalPostForm().)

Only confusing if you're used to D7. ;-) https://www.drupal.org/node/2087433

dawehner’s picture

Going forward, let's please stop doing individual module conversions and do larger chunks at once.

Where was this decision made? While I agree having this particular research issue can be helpful, I am not convinced that going through tests individually (on a per module and for the easy modules) is a bad idea.

xjm’s picture

Edit: moving my comments to the other issue.

xjm’s picture

@dawehner, also, I guess #15 was on the wrong issue so let's discuss on #2735005: Convert all Simpletest web tests to BrowserTestBase (or UnitTestBase/KernelTestBase).

dawehner’s picture

Applied the strategy from #2735005-74: Convert all Simpletest web tests to BrowserTestBase (or UnitTestBase/KernelTestBase) using the small script below.

This is not complete yet.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

xjm’s picture

klausi’s picture

Status: Needs work » Needs review

We need to trigger the testbot for @dawehner's patch.

Status: Needs review » Needs work

The last submitted patch, 18: 2770921-18.patch, failed testing.

klausi’s picture

@xjm: opened #2784263: Move WTB::assertCacheContext() to AssertPageCacheContextsAndTagsTrait to consolidate assertCacheContext() with Simpletest.

dawehner’s picture

I adapted the script to

  • Iterate over all core modules
  • Allow to exclude tests by pattern
  • Excluded a few obvious ones

This produced the following patch. IMHO we should include the conversion script into the patch, so we can see what people changed in it.

dawehner’s picture

Status: Needs work » Needs review

.

Status: Needs review » Needs work

The last submitted patch, 24: 2770921-24.patch, failed testing.

dawehner’s picture

Status: Needs work » Needs review
FileSize
372.71 KB

Forgot to pull recently.

Status: Needs review » Needs work

The last submitted patch, 27: 2770921-27.patch, failed testing.

dawehner’s picture

Issue summary: View changes
Status: Needs work » Needs review
FileSize
524.62 KB
183.86 KB
  • Ensured to always keep the traits
  • Excluded some more tests
dawehner’s picture

Excluded some individual tests more

dawehner’s picture

Excluded some more

The last submitted patch, 29: 2770921-30.patch, failed testing.

The last submitted patch, 30: 2770921-32.patch, failed testing.

dawehner’s picture

Excluded some more

The last submitted patch, 31: 2770921-31.patch, failed testing.

dawehner’s picture

This time we also convert tests in folders ... which includes ALL of system.

dawehner’s picture

Some more exclusions.

The last submitted patch, 34: 2770921-34.patch, failed testing.

dawehner’s picture

Some more.

The last submitted patch, 36: 2770921-36.patch, failed testing.

The last submitted patch, 37: 2770921-37.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 39: 2770921-39.patch, failed testing.

dawehner’s picture

Status: Needs work » Needs review
FileSize
681.66 KB
2.41 KB

* Applied the patch from #2784537: Add legacy assertFieldByXPath()/assertNoFieldByXPath() method for browser tests to fix a good couple of test failures
*Fixed emails in BrowserTestBase
* A couple of more exclusions

Status: Needs review » Needs work

The last submitted patch, 43: 2770921-43.patch, failed testing.

dawehner’s picture

dawehner’s picture

Status: Needs work » Needs review

.

Status: Needs review » Needs work

The last submitted patch, 45: 2770921-45.patch, failed testing.

klausi’s picture

Oh, exciting! Thanks for the painful and hard work dawehner. If we can make this green by just converting everything that passes right now I would RTBC it. Even more super cool would be to get this ready before 8.2 releases :)

dawehner’s picture

Status: Needs work » Needs review
FileSize
679.07 KB
8.47 KB

Let's exclude all the other ones.

// Fetch all failing test classes ...
trs = jQuery('tr.pift-ci-class').not('.pift-ci-all-pass').map(function (key, value) { return jQuery(value).find('td')[2].innerHTML; });
jQuery(trs).each(function (key, value) { console.log(value.split('.')[1]); });

Status: Needs review » Needs work

The last submitted patch, 49: 2770921-49.patch, failed testing.

dawehner’s picture

Status: Needs work » Needs review
FileSize
551.9 KB
11.9 KB

Well, let's do it like this

Status: Needs review » Needs work

The last submitted patch, 51: 2770921-51.patch, failed testing.

klausi’s picture

Hm strange, even BrowserTestBaseTest is failing with this patch. Can you check if it passes for you locally with the 2 new test methods it has?

dawehner’s picture

dawehner’s picture

dawehner’s picture

Status: Needs work » Needs review
FileSize
553.63 KB
1.04 KB

Let's see how many are left now.

Status: Needs review » Needs work

The last submitted patch, 56: 2770921-56.patch, failed testing.

dawehner’s picture

dawehner’s picture

Status: Needs work » Needs review

This one could be green

Status: Needs review » Needs work

The last submitted patch, 58: 2770921-58.patch, failed testing.

dawehner’s picture

Status: Needs work » Needs review

It is green!

Status: Needs review » Needs work

The last submitted patch, 58: 2770921-58.patch, failed testing.

naveenvalecha’s picture

Status: Needs work » Needs review
+++ b/browsertest-convert.php
@@ -0,0 +1,553 @@
+  'SimpleTestBrowserTest',
...
+  'SimpleTestBrowserTest',

Only a question : Why are we skipping the same test two times ?
Retested for 8.2.x, patch not applied, movig back to N/R

dawehner’s picture

Only a question : Why are we skipping the same test two times ?
Retested for 8.2.x, patch not applied, movig back to N/R

The reason why this happened is because I organized them by issue, which blocks a certain test to be converted. There are tests which have multiple blockers, which is the reason they appear twice in there.

Retested for 8.2.x, patch not applied, movig back to N/R

I think we should commit it to 8.3.x and then try to redo the same in 8.2.x

Mile23’s picture

Status: Needs review » Needs work

+1 on limiting to 8.3.x and then moving to 8.2.x if possible. 8.2.x changes would probably be non-disruptive, and would be allowed in beta since this is a testing improvement.

  1. diff --git a/browsertest-convert.php b/browsertest-convert.php
    

    Should we keep this script in the repo? Maybe under core/scripts so contrib can use it? If we're going to do that, it'll need some more polish.

  2. +++ b/core/modules/action/tests/src/Functional/ActionListTest.php
    @@ -1,15 +1,15 @@
    diff --git a/core/modules/aggregator/tests/src/Functional/AggregatorTestBase.php b/core/modules/aggregator/tests/src/Functional/AggregatorTestBase.php
    
    diff --git a/core/modules/aggregator/tests/src/Functional/AggregatorTestBase.php b/core/modules/aggregator/tests/src/Functional/AggregatorTestBase.php
    new file mode 100644
    
    new file mode 100644
    index 0000000..a6c9c32
    
    index 0000000..a6c9c32
    --- /dev/null
    
    --- /dev/null
    +++ b/core/modules/aggregator/tests/src/Functional/AggregatorTestBase.php
    

    There are a bunch of these base classes in the patch that seem to come from nowhere. I talked with @dawehner in IRC about it and they're being copied for backwards compatibility, which is good.

klausi’s picture

Those new test bases should be almost similar copies from Simpletest, right? Why does the patch not show them as new copy/move? Can you roll the patch again with git --find-copies or -M25% or something? If the code is the same as in the Simpletest classes can we share it via traits?

browsertest-convert.php should not stay in the patch, but we can keep it until the patch is fully polished.

dawehner’s picture

Those new test bases should be almost similar copies from Simpletest, right? Why does the patch not show them as new copy/move? Can you roll the patch again with git --find-copies or -M25% or something?

yeah they are copies. I'll try it next time I roll the patch. Let's get first the two other blockers in.

Those new test bases should be almost similar copies from Simpletest, right? Why does the patch not show them as new copy/move? Can you roll the patch again with git --find-copies or -M25% or something? If the code is the same as in the Simpletest classes can we share it via traits?

If we really think its worth sharing it via traits I would hope we don't want to do it as part of this issue. This cannot be achieved via some script, which is kinda the point of this issue :)

klausi’s picture

Assigned: Unassigned » klausi
klausi’s picture

Assigned: klausi » Unassigned
Status: Needs work » Needs review
FileSize
246.05 KB
215.56 KB

Added some more exceptions to the script:
* do not convert Views tests, Installer tests, Module tests etc.
* do not copy Traits because we just want to reuse them from phpunit (this will likely cause some fails because I forgot use statements)
* mark old test base classes as deprecated. That way Git can pick up the moved file in the diff and we just see the actual changes.

klausi’s picture

Status: Needs review » Needs work

The last submitted patch, 69: btb-2770921-69.patch, failed testing.

klausi’s picture

Status: Needs work » Needs review
FileSize
245.18 KB
2.98 KB

Fixed use statement and conversion script.

Status: Needs review » Needs work

The last submitted patch, 72: btb-2770921-72.patch, failed testing.

dawehner’s picture

Status: Needs work » Needs review
FileSize
246.55 KB
1.92 KB

Let's fix stuff ... :)

Status: Needs review » Needs work

The last submitted patch, 74: 2770921-74.patch, failed testing.

dawehner’s picture

Status: Needs work » Needs review
FileSize
2.86 KB
245.73 KB

Meh, another failing one.

Status: Needs review » Needs work

The last submitted patch, 76: 2770921-76.patch, failed testing.

dawehner’s picture

Status: Needs work » Needs review
FileSize
247.04 KB

There we go, I forgot to add #76 as well

Status: Needs review » Needs work

The last submitted patch, 78: 2770921-78.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 80: 2770921-80.patch, failed testing.

klausi’s picture

Status: Needs work » Needs review

Cool this is green now.

What do we do with this patch now? Make an announcement that we will commit this on for example Sept 28th to 8.3.x and 8.2.x? Or is it to late for 8.2?

If it is too late for 8.2 how do we proceed? Make up another random date after 8.2 is released? Or do we have to wait for the next code freeze phase in March 2017 for 8.3?

I propose that we do this before 8.2 is released to keep momentum, would be cool to get some core committer feedback.

dawehner’s picture

Let' discuss that on the main issue.

xjm’s picture

So both https://www.drupal.org/pift-ci-job/463727 and https://www.drupal.org/pift-ci-job/463729 include something like:
Behat\Mink\Exception\ExpectationException: Link with label ||m`>&!s found.

Sounds like a classic random machine name vs. random string bug; is there any chance we're surfacing an issue like that?

claudiu.cristea’s picture

Status: Needs review » Postponed
Related issues: +#2808085: Pipe char in locators break Mink and Symfony element search

Random test failures from #80 seems to be caused by #2808085: Pipe char in locators break Mink and Symfony element search. Occasionally the random string generator adds the pipe in output, then this text/label is used in Mink locators.

Postponing on #2808085: Pipe char in locators break Mink and Symfony element search.

Status: Needs review » Needs work

The last submitted patch, 86: browsertest-convert-2770921-86.patch, failed testing.

klausi’s picture

klausi’s picture

Status: Needs review » Needs work

The last submitted patch, 89: browsertest-convert-2770921-90.patch, failed testing.

klausi’s picture

Aha, this is the same random test fail as experienced earlier in #2784537: Add legacy assertFieldByXPath()/assertNoFieldByXPath() method for browser tests. Let's find out there what causes this.

klausi’s picture

Status: Needs work » Needs review
FileSize
260.09 KB
9.84 KB

Status: Needs review » Needs work

The last submitted patch, 92: browsertest-convert-2770921-92.patch, failed testing.

The last submitted patch, 92: browsertest-convert-2770921-92.patch, failed testing.

klausi’s picture

Yay, it's green! Queuing a bunch of test runs to make sure we have indeed eliminated the random test fails. Please also review all the issues I linked further above.

dman’s picture

That is looking *very* green! Good stuff. Applied well for me, and I'm spot-checking a few of the small number tests I'm actually already familiar with.
What can we do to help next? (barring running hours of re-tests locally). I had a look, but can't see any action/help I can provide on the related issues I could see whilst this mega-change is pending here.

klausi’s picture

dawehner’s picture

Nice!!!

klausi’s picture

klausi’s picture

klausi’s picture

Title: Testing issue: Convert WTB to BTB by just moving classes and changing use statements » Testing issue: Convert WTB to BTB by just moving classes, changing use statements adding traits
Issue summary: View changes

The direction of this issue has changed slightly, we are now also adding traits to converted test classes because we don't want to make BrowserTestBase larger.

klausi’s picture

Merged in 8.3.x, added 2 tests to the conversion script that cannot be automatically converted right now.

#2784537: Add legacy assertFieldByXPath()/assertNoFieldByXPath() method for browser tests is still pending, let's hope it gets committed soon.

klausi’s picture

And the last one got committed. Simple reroll, no other changes.

Once this is green we can remove the conversion script from the patch.

We should also announce a date now when we want to commit this large phpunit conversion chunk.

Status: Needs review » Needs work

The last submitted patch, 106: browsertest-convert-2770921-106.patch, failed testing.

klausi’s picture

Status: Needs work » Needs review

Looks like a random fail in Drupal\page_cache\Tests\PageCacheTest, queuing all environments again just to make sure.

klausi’s picture

We have one Drupal\aggregator\Tests\FeedLanguageTest fail complaining that Drupal is already installed (race condition with parallel tests?) and one Drupal\Tests\ckeditor\FunctionalJavascript\AjaxCssTest fail on PHP 7.1.x-dev which is an experimental environment.

So this should be good to go, can somebody double check and RTBC this?

claudiu.cristea’s picture

We have one Drupal\aggregator\Tests\FeedLanguageTest fail complaining that Drupal is already installed (race condition with parallel tests?) and one Drupal\Tests\ckeditor\FunctionalJavascript\AjaxCssTest fail on PHP 7.1.x-dev which is an experimental environment.

@klausi, let's exclude them from this conversion and open 2 follow-ups just to be sure we're not breaking anything.

claudiu.cristea’s picture

Hm. FeedLanguageTest was not converted and AjaxCssTest is a JS test. Ignore my comment.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

Either we have an impressive test coverage in Drupal, or we have converted a hell lot of tests in this patch, or both :)
I really like how it covers tests from a lot of subsystems. This should give us higher confidence.

I went through the changes with phpstorm and couldn't see anything wrong. We really just move files / move base classes around.

klausi’s picture

Thanks!

As mentioned in #2807237: PHPUnit initiative we will also announce a date when we commit this conversion to 8.3.x. I propose February 27th 2017, shorty before 8.3.0-rc1 is released. I will poke the core committers now to get a date fixed and announced.

dawehner’s picture

A monday would be a good idea IMHO, as there is a lot of time during the week to fix stuff. To be honest though given that RC is this week people might be more stressed out. Maybe a week earlier, aka. the 20th would be a better time.

klausi’s picture

catch also said a week before RC would be good, so that would match Feb 20th as dawehner proposed. That also works for me.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

xjm’s picture

That timeline sounds good to me as well! How exciting.

Assuming we go forward with this plan, we can announce it after we release the alpha.

jibran’s picture

Do we want to enforce no more WTB in new patch policy after this is fixed?

dawehner’s picture

@jibran
I think we should discuss in a separate issue.

mpdonadio’s picture

klausi’s picture

Title: Testing issue: Convert WTB to BTB by just moving classes, changing use statements adding traits » Feb 20th: Convert chunk of WTB to BTB by just moving classes, changing use statements adding traits
Version: 8.4.x-dev » 8.3.x-dev
Issue summary: View changes

OK, so let's aim for Feb 20th. And this will go into the 8.3.x branch.

Drafting an announcement text in the issue summary. What else should we mention in the announcement?

mpdonadio’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
216.76 KB
5.69 KB

A datetime.module class snuck into a recent patch. This removes it (I just hand edited #106 and removed the hunks); will be part of #2780063: Convert web tests to browser tests for datetime and datetime_range modules.

mpdonadio’s picture

Issue tags: +DrupalCampNJ2017

Keep forgetting to tag issues.

klausi’s picture

One minor thing:

+++ b/core/modules/content_translation/tests/src/Functional/ContentTranslationUITestBase.php
@@ -600,7 +599,7 @@ public function doTestChangedTimeAfterSaveWithoutChanges() {
     // Test only entities, which implement the EntityChangedInterface.
-    if ($entity instanceof EntityChangedInterface) {
+    if ($entity->getEntityType()->isSubclassOf('Drupal\Core\Entity\EntityChangedInterface')) {
       $changed_timestamp = $entity->getChangedTime();

this looks like an unrelated change, why is it necessary?

klausi’s picture

Sorry, that change has been introduced before in one of my earlier patches. Reverted.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

I went through the patch again and couldn't find anything else than pure moving. You have good eyes klausi!

xjm’s picture

I just learned that Feb. 20 is a US holiday and many businesses will be closed. I wonder if we should do this Feb. 21 instead?

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 125: browsertest-convert-2770921-125.patch, failed testing.

klausi’s picture

Status: Needs work » Needs review
FileSize
216.19 KB
2.88 KB

Rerolled, interdiff is resolving the merge conflicts and rolling back what git wrongly merged.

klausi’s picture

Title: Feb 20th: Convert chunk of WTB to BTB by just moving classes, changing use statements adding traits » Feb 21st: Convert chunk of WTB to BTB by just moving classes, changing use statements adding traits
Issue summary: View changes

@xjm: yes, Feb 21st also works!

klausi’s picture

Issue summary: View changes

fixing one more date.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

It is green, as expected.

mpdonadio’s picture

klausi’s picture

Only 8 days to go, will ping the release managers again on IRC to get the announcement out at https://groups.drupal.org/core before it is too late.

klausi’s picture

Only 4 days to go, I guess it does not make sense anymore to put out the announcement because nobody could react to it anyway over the weekend?

So I propose that we change the announcement to just inform people after the fact that stuff has been converted. I will change the proposed text after this has been committed on Tuesday :)

klausi’s picture

Just saw a response from xjm on IRC that I have now permission to publish announcements myself on https://groups.drupal.org/core . Published the announcement, better late then never! https://groups.drupal.org/node/516229

xjm’s picture

Thanks @klausi! Very sorry I was not able to follow up sooner; it got lost in the flood of things over the past week.

I am super excited!

xjm’s picture

BTW keeping this issue RTBC so it gets nightly retesting was a good choice; it helps us make sure that we are no longer introducing any random failures that are not accounted for by known issues. I can confirm everything on the issue so far is also present in HEAD because I get emails every time a core branch fails and I've seen them all before. :P

xjm’s picture

klausi’s picture

Thanks a lot for watching out xjm! I really think we should immediately disable all random test failure prone test cases to prevent you getting the same fail emails every day.

jibran’s picture

I think it is 21st February in every timezone now. :D *fingers crossed*

xjm’s picture

@jibran Well, for the Americans, committing patches at 2/4/5a is probably not a great idea. ;)

Keeping an eye out now on the queued tests. FWIW, I also confirmed that the conversion does not conflict with our two RC-sensitive BC issues (or reverts thereof), #2751325: All serialized values are strings, should be integers/booleans when appropriate and #2075889: Make Drupal handle incoming paths in a case-insensitive fashion for routing, since both use our modern testing APIs already.

catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.4.x and cherry-picked to 8.3.x. Thanks!

  • catch committed 4c64f7b on 8.4.x
    Issue #2770921 by dawehner, klausi, Mile23, mpdonadio, xjm, claudiu....
vaplas’s picture

FileSize
150.16 KB

Hooray! This patch made my day!

mpdonadio’s picture

Think there is a weird regression with the conversion, https://www.drupal.org/node/2745123#comment-11949935

Will post followup, but I am not 100% sure what is happening to make a real bug report yet, other than something potentially different in the way test teardown works that make the language manager unhappy.

xjm’s picture

Issue tags: +8.3.0 release notes

This way merits an RN mention.

xjm’s picture

@mpdonadio, did you end up posting that followup? Sounds potentially important?

mpdonadio’s picture

#148, no I think @daehner, @klausi, and @alexpott are of the opinion that this isn't really related. It has more of the other issue doing something that it shouldn't be.

Status: Fixed » Closed (fixed)

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