CommentFileSizeAuthor
#18 interdiff_convert-from-simpletests-to-phpunit-tests-3055728-13-18.txt1.89 KBw.drupal
#18 convert-from-simpletests-to-phpunit-tests-3055728-18.patch51.74 KBw.drupal
#13 convert-from-simpletests-to-phpunit-tests-3055728-13.patch52.06 KBw.drupal

Comments

AdamPS created an issue. See original summary.

w.drupal’s picture

w.drupal commented
Assigned: Unassigned » w.drupal
Status: Active » Needs work
adamps’s picture

adamps commented
Version: 8.x-1.x-dev » 8.x-2.x-dev

w@drupal great that you are working on this. I will commit some other issues soon (including #3055868: Delete entity load/delete wrappers) that will make changes to test files. It might be easier for you if you wait 2 or 3 weeks before starting this one.

w.drupal’s picture

w.drupal commented

@AdamPS ok, let me know when I can start

w.drupal’s picture

w.drupal commented

@AdamPS, is it good time to start process this issue?

adamps’s picture

adamps commented

w@drupal thanks for checking. Don't worry I hadn't forgotten. It's nearly the right time.

First I will offer @TR the chance to fix #3037140: Fix coding standards. He has been waiting longer, plus he says that the automatic tool to convert the test will work better if coding standards are fixed - so it should make your life easier. If he doesn't reply by 20th July that you can go. If he posts a patch it might be a little longer, but still soon. I suggest that you follow the other issue.

Thanks!

adamps’s picture

adamps commented

w@drupal OK to go ahead with fixing this one, many thanks.

I will try to avoid any other commits that change tests for as long as I can to save you from merging/re-rolls. But for sure if you can start soon then it will help.

adamps’s picture

adamps commented
Status: Needs work » Postponed

OK no response here and another developer is keen to work on #3037140: Fix coding standards so let's do that one first. It should only be 1-2 weeks then can work on this one again.

w.drupal’s picture

w.drupal commented
Status: Postponed » Needs work

@AdamPS i'm in progress with this issue. I think I'll finish this Friday

adamps’s picture

adamps commented

Great news - in that case you have priority, thanks.

w.drupal’s picture

w.drupal commented

@AdamPS

This test fails phpunit --filter testSendFail "/app/web/modules/contrib/simplenews/tests/src/Functional/SimplenewsSendTest.php" but as I see from https://www.drupal.org/project/simplenews/issues/3053223 this is expected. Should we leave it as is?

adamps’s picture

adamps commented

That test is expected to fail against D8.7.2 but succeed against D8.8. I have set the default Drupal.org testing branch to D8.8 - provided that works we can commit.

w.drupal’s picture

w.drupal commented
StatusFileSize
new convert-from-simpletests-to-phpunit-tests-3055728-13.patch52.06 KB

I still have one failure:

There was 1 failure:

1) Drupal\Tests\simplenews\Functional\SimplenewsI18nTest::testNewsletterIssueTranslation
Failed asserting that 3 matches expected 0.

/app/web/core/tests/Drupal/Tests/BrowserTestBase.php:690
/app/web/core/tests/Drupal/KernelTests/AssertLegacyTrait.php:61
/app/web/modules/contrib/simplenews/tests/src/Functional/SimplenewsI18nTest.php:144

Seems like emails don't appear in the "test_mail_collector" state after \Drupal::service('cron')->run();

Any ideas why?

adamps’s picture

adamps commented

There are some random failures - I have scheduled a retest

w.drupal’s picture

w.drupal commented

Doesn't looks like a random. It's reproduces on the every local test running
phpunit --filter testNewsletterIssueTranslation "/app/web/modules/contrib/simplenews/tests/src/Functional/SimplenewsI18nTest.php"

adamps’s picture

adamps commented

There were two failures the first time it ran, and one was random so now only one.

adamps’s picture

adamps commented

Thanks for your work on this patch, you clearly have made a lot of progress.

I think the problem is that you are calling \Drupal::service('cron')->run(); directly which means no URL is set and so simplenews_cron() calls simplenews_assert_uri() and it fails.

I suggest the fix is to keep the call $this->cronRun(); and add use CronRunTrait;

w.drupal’s picture

w.drupal commented
Status: Needs work » Needs review
StatusFileSize
new convert-from-simpletests-to-phpunit-tests-3055728-18.patch51.74 KB
new interdiff_convert-from-simpletests-to-phpunit-tests-3055728-13-18.txt1.89 KB

Indeed it is this problem. Fixed now

adamps’s picture

adamps commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +Plan to commit

Many thanks looks good to me. I will commit after giving other maintainers a chance to review.

  • AdamPS committed a403973 on 8.x-2.x authored by w.drupal 
    Issue #3055728 by w.drupal, AdamPS: Convert from Simpletests to PHPUnit...
adamps’s picture

adamps commented
Assigned: w.drupal » Unassigned
Status: Reviewed & tested by the community » Fixed
adamps’s picture

adamps commented
Issue tags: -Plan to commit
jcnventura’s picture

jcnventura
Primary language English
commented
Status: Fixed » Active

It seeems this missed one test in modules/simplenews_demo/src/Tests/SimplenewsDemoTest.php

w.drupal’s picture

w.drupal commented
Assigned: Unassigned » w.drupal
Status: Active » Needs review

i've created child issue for this test. Patch will be uploaded there.

adamps’s picture

adamps commented
Status: Needs review » Fixed

Thanks in that case we can set this issue back to fixed.

Status: Fixed » Closed (fixed)

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