Create contrib Update test for legacy to semver releases

Agreed on splitting this out. Seems it should be at least major though, since the parent is critical and this is essential functionality we quickly want coverage of.

A few concerns/nits for 3127168-2-do-not-test.patch:

1. +++ b/core/modules/update/tests/modules/update_test/semver_test.1.0-legacy-unsupported.xml
   @@ -168,5 +168,83 @@
   +<release>
   
   +++ b/core/modules/update/tests/modules/update_test/semver_test.1.0.xml
   @@ -168,5 +168,83 @@
   +<release>
   
   +++ b/core/modules/update/tests/modules/update_test/semver_test.9.1.0.xml
   @@ -0,0 +1,250 @@
   +<release>
   

   Missing a space in front.

2. +++ b/core/modules/update/tests/modules/update_test/semver_test.1.0-legacy-unsupported.xml
   @@ -168,5 +168,83 @@
   +    <tag>SEMVER_TEST-8-x-7-1</tag>
   ...
   +    <tag>SEMVER_TEST-8-x-7-1-beta1</tag>
   ...
   +    <tag>SEMVER_TEST-8-x-7-1-alpha1</tag>
   ...
   +    <tag>SEMVER_TEST-8-x-7-0</tag>
   ...
   +    <tag>SEMVER_TEST-8-x-7-0-beta1</tag>
   ...
   +    <tag>SEMVER_TEST-8-x-7-0-alpha1</tag>
   
   +++ b/core/modules/update/tests/modules/update_test/semver_test.1.0.xml
   @@ -168,5 +168,83 @@
   +    <tag>SEMVER_TEST-8-x-7-1</tag>
   ...
   +    <tag>SEMVER_TEST-8-x-7-1-alpha1</tag>
   ...
   +    <tag>SEMVER_TEST-8-x-7-0</tag>
   ...
   +    <tag>SEMVER_TEST-8-x-7-0-beta1</tag>
   ...
   +    <tag>SEMVER_TEST-8-x-7-0-alpha1</tag>
   
   +++ b/core/modules/update/tests/modules/update_test/semver_test.9.1.0.xml
   @@ -0,0 +1,250 @@
   +    <tag>SEMVER_TEST-8-x-8-1</tag>
   ...
   +    <tag>SEMVER_TEST-8-x-8-1-alpha1</tag>
   ...
   +    <tag>SEMVER_TEST-8-x-8-0</tag>
   ...
   +    <tag>SEMVER_TEST-8-x-8-0-beta1</tag>
   ...
   +    <tag>SEMVER_TEST-8-x-8-0-alpha1</tag>
   

   #3113798: Remove unused (and generally wrong) <tag> markup from Update module test XML fixtures or fix.

3. +++ b/core/modules/update/tests/modules/update_test/semver_test.1.0.xml
   index 00000000000..c4f8a66fccf
   --- /dev/null
   
   --- /dev/null
   +++ b/core/modules/update/tests/modules/update_test/semver_test.9.1.0.xml
   
   +++ b/core/modules/update/tests/modules/update_test/semver_test.9.1.0.xml
   +++ b/core/modules/update/tests/modules/update_test/semver_test.9.1.0.xml
   @@ -0,0 +1,250 @@
   
   @@ -0,0 +1,250 @@
   +<?xml version="1.0" encoding="utf-8"?>
   +<project xmlns:dc="http://purl.org/dc/elements/1.1/">
   

   Until we prioritize #3115435: Make clear why each XML update.module fixture is created the way it is, every update manager issue that adds new test fixtures is adding technical debt since it's making more work for ourselves at #3115435. Since this is a brand new fixture for this issue, let's document it as part of this issue.

4. +++ b/core/modules/update/tests/modules/update_test/semver_test.9.1.0.xml
   @@ -0,0 +1,250 @@
   +    <!-- This release is not in a supported branch; therefore it should not be recommended. -->
   +    <name>Semver Test 9.2.0</name>
   +    <version>9.2.0</version>
   +    <tag>9.2.0</tag>
   +    <status>published</status>
   +    <release_link>http://example.com/semver_test-9-2-0-release</release_link>
   +    <download_link>http://example.com/semver_test-9-2-0.tar.gz</download_link>
   +    <date>1584195300</date>
   +    <terms>
   +      <term><name>Release type</name><value>New features</value></term>
   +      <term><name>Release type</name><value>Bug fixes</value></term>
   +    </terms>
   +  </release>
   

   Any reason this one is indented 2 spaces, every other one only 1? Oh, at the bottom of the file there are a few more indented 2 spaces. Can we pick an indentation and be consistent about it?

5. +++ b/core/modules/update/tests/src/Functional/UpdateSemverContribTest.php
   @@ -49,4 +49,69 @@ protected function setProjectInstalledVersion($version) {
   +   * Test updates from legacy versions to the semver versions.
   

   Tests?

6. +++ b/core/modules/update/tests/src/Functional/UpdateSemverContribTest.php
   @@ -49,4 +49,69 @@ protected function setProjectInstalledVersion($version) {
   +        foreach ($installed_versions as $install_version) {
   

   as $installed_version might make more sense for the scope of this foreach.

7. +++ b/core/modules/update/tests/src/Functional/UpdateSemverContribTest.php
   @@ -49,4 +49,69 @@ protected function setProjectInstalledVersion($version) {
   +            if ($legacy_major === 8) {
   +              continue;
   +            }
   

   A comment for why we continue/skip this case would be helpful.

8. +++ b/core/modules/update/tests/src/Functional/UpdateSemverContribTest.php
   @@ -49,4 +49,69 @@ protected function setProjectInstalledVersion($version) {
   +            // @todo Determine if both 8.0.0 and 8.0.1 should be expected as
   +            // "Also available" releases in
   +            // https://www.drupal.org/project/node/3100115.
   

   I thought our convention was:

   "@todo In https://.../node/x determine ..." and to indent subsequent lines of the same @todo.

Thanks,
-Derek

Opened a merge request , there are no changes from the patch file yet though.

Two fixtures were missed in the original conversion of the patch. With those restored the test was passing locally. And then I did the cleanup from #4.

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Very nice additional test @quietone!

Added a small nitpicky change but additional test coverage appears fine to me.

Committed and pushed 6c1d6a5b5e to 11.x and 6313a810fb to 10.3.x. Thanks!