Make sure Simple Google Maps is fully Drupal 9 compatible

Let's expand this issue to be "Make this module fully compliant with Drupal 9".

Checked with drupal-check and this is the only error:

------ ------------------------------------------------------------
Line src\Tests\SimpleGmapTest.php
------ ------------------------------------------------------------
49 Call to deprecated method entityManager() of class Drupal.
------ ------------------------------------------------------------

Built a patch for the one open problem.

Update the patch with core_version_requirement to support Drupal 9

Patch is failing. Changing to correct status.

Rechecked against Drupal 8.9 and got following errors:

[2K 3/3 [============================] 100%

------ -------------------------------------------------------------------------------------------------------
Line src\Tests\SimpleGmapTest.php
------ -------------------------------------------------------------------------------------------------------
17 Class Drupal\simple_gmap\Tests\SimpleGmapTest extends deprecated class Drupal\simpletest\WebTestBase.
45 Call to method setUp() of deprecated class Drupal\simpletest\WebTestBase.
49 Call to deprecated method entityManager() of class Drupal.
------ -------------------------------------------------------------------------------------------------------

[ERROR] Found 3 errors

Hi Folks

I applied the patch in #8 and did some changes an looks like we are ready for Drupal 8.8 and 9

Tested with Drupal 8.8

Except that the test doesn't pass.

Test case is failing because BrowserTestBase doesn't have drupalPostAjaxForm() method.

Replaced drupalPostAjaxForm() with drupalPostForm().

I also noticed while running tests on local that tests throwing warning for "\Drupal::service('entity.definition_update_manager')->applyUpdates();" as this is depractaed.

I am wondering if below is really required?

\Drupal::service('router.builder')->rebuild();
    \Drupal::service('entity.definition_update_manager')->applyUpdates();

Patch in #13 looks good to me.

➜  d9 git:(8.8.x) ✗ ./vendor/mglaman/drupal-check/drupal-check --drupal-root=./ modules/contrib/simple_gmap

 3/3 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%


 [OK] No errors

Not really sure if "\Drupal::service('entity.definition_update_manager')->applyUpdates();" is needed or not as per comment above.

If you are getting an error when running the test locally, that something is deprecated, it is probably a good idea to remove it. Setting to Needs Work for that. Thanks!

Thanks Rohit, #16 looks good to me.

This whole patch seems wrong, actually:
- Tests that are BrowserTestBase should be under (module directory)/tests/src
- Tests that use Ajax should be converted to JavascriptTestBase, not BrowserTestBase.

I'm not as familiar with this test for the Drupal 8/9 version of the module, but in the Drupal 7 version, the test basically was there so that we could run it interactively and look at the results to see the maps visible. @martin107 -- not sure if you use the Drupal 8 test that way? Assigning this to you to reply.

In any case, this is wrong. See https://www.drupal.org/docs/8/phpunit for more info on phpunit tests in Drupal 8/9.

There's a different patch for the test on #2913307: Prepare Tests for upgrade. -- not done but seems to be pointing in a better direction. Maybe we should limit this patch to the line in the .info.yml file and continue on the test in that other issue? Otherwise close that as a duplicate of this one.

@martin_107 is now working on the tests on the other issue. So let's remove the tests from this patch, and also (since the title is "make sure fully compatible" on this issue, postpone this issue until the other one is done.

Cool, the tests are done. And it seems that there is nothing else to do. So the only missing thing is a new release, now.

So the only missing thing is a new release,

So I have looked through several articles online about preparing for Drupal9

They seem to repeat a simple check list

1) Make sure no errors are reported by drupal-check.
2) Sort out the simple_gmap.info.yml with respect to "core_version_requirement: ^8 || ^9"
3) ...release?

I am pausing with step 3, is Drupal8.9 frozen yet? I could not immediately gleam that from the articles.

.. that is are we currently just in sync with Drupal9's current status?

If things are not frozen then there could be future changes in Core for which we must adjust.

I agree a release seems appropriate - as a signal to a broader audience that the readiness of the module has improved.

I am not sure of the wording for the release ...

Are we "Drupal9 aligned" or "Drupal9 compatibility guaranteed".

What would other consumers of this module like to be told in the release notes.

In terms of planning, feedback is welcome, and then perhaps I will trigger a release early next week?

Add to checklist: run tests against D9? and no D9 is not frozen.

But there won‘t be any new deprecations in D8. So we have fixed everything

Thank you both for the feedback

Carefully parsing the language, from the internet and what you have said

.... there are two separable points.

1) Not frozen.

2) Drupal 8.9 by design won't carry any new deprecation....

With those objectives there is still plenty of possibility for unforeseen screw ups.

So I will rerun test on D9 and then the tone of the text I use in the new release notes will speak of a "release candidate 1" as far as Drupal9 compatibility goes.

That seems the best way of preventing needless car crashes for site owners, who are not following the comings and goings of core development.

Fingers crossed, I have time for release on Sunday night.

That sounds good to me! Anyway I think the branch/version can still say 8.x-something, and with the changes already committed (??) to the info file the new release should be compatible with 8.x.several, plus 9.x.... Actually, I amend my suggestion in #23 ... I suggest going to the Automated tests tab of this project and testing against the oldest version of 8.x we want to support, and also against 9.x, before making release. Info file currently says:
core_version_requirement: ^8 || ^9

It probably should be more specific on 8.x? https://www.drupal.org/node/3070687

Given that we are now pre-compatible with D9, I have bumped the major number and released

https://www.drupal.org/project/simple_gmap/releases/8.x-2.0

I will be updateing the front page and tweaking the automated test strategy, next.

Thank you all for your patience ..most of the delays related to this issue have been on my account.