Issue #3227518: Never show a "Not supported!" version as "Recommended"
Merge request reports
Activity
1 <?xml version="1.0" encoding="utf-8"?> 2 <!-- 3 This XML file is the exact same as the file drupal.1.xml except 4 'supported_branches' in this file does not contain the '8.0.' branch. Therefore, 5 all the releases that start with '8.0.' are in an unsupported branch. 
I did a comparison between this file and drupal.1.xml. drupal.1.xml at least also has
Drupal 8.2.0release so maybe that should be added here? There might be other differences. If the intention of this issue is thesupported_branchesis the only difference lets update this file so that is true
1 <?xml version="1.0" encoding="utf-8"?> 2 <!-- 3 This XML file is the exact same as the file semver_test.1.xml except 4 'supported_branches' in this file does not contain the '8.0.' branch. Therefore, 5 all the releases that start with '8.0.' are in an unsupported branch. added 2 commits
248 248 $this->assertUpdateTableTextContains($unsupported_version); 249 249 $this->assertUpdateTableElementContains('error.svg'); 250 250 $this->assertUpdateTableTextContains('Release not supported: Your currently installed release is now unsupported, and is no longer available for download. Disabling everything included in this release or upgrading is strongly recommended!'); 251 $this->assertVersionUpdateLinks($new_version_label, $newer_version); 251 if ($newer_version !== NULL) { -
Since NULL is the default value for this parameter in the this assert then if this value not NULL then we intentionally sent a value in to be tested. This is always a literal string from a test case never user input. So I think if we have anything beside the default value we should pass that onto
assertVersionUpdateLinks().If we want to change anything here I think it would be in the
elseto have$this->assertNotEmpty($newer_version);Because that would problem in the test logic
-
Why we would add
$this->assertNotEmpty($newer_version);inelse?Edited by Kunal Sachdev -
@kunal.sachdev yes I think you are correct.
added 43 commits
-
d831c763...afc5a87e - 40 commits from branch
project:9.3.x - 8948b513 - Merge branch '9.3.x' into 3227518-never-show-a
- b151c1a2 - reverted changes in the drupalci file
- 298cc5a6 - changed the error message according to the conditions
Toggle commit list-
d831c763...afc5a87e - 40 commits from branch
added 1 commit
- 9ea36272 - added condition for checking also available version
added 9 commits
-
9ea36272...59d8ab1f - 7 commits from branch
project:9.3.x - 39960219 - Merge branch '9.3.x' into 3227518-never-show-a
- 55cd579f - added test coverage for the also available case
-
9ea36272...59d8ab1f - 7 commits from branch
added 1 commit
- a5a00051 - Revert "changed order of release in fixture"
added 27 commits
-
a5a00051...81690f84 - 12 commits from branch
project:9.3.x - d2d94d41 - added test coverage to prove that this problem currently exists
- bd732733 - added comment for the new test coverage
- abc24758 - phpcbf
- ecc5a99f - added new variable to determine the release is supported
- b35427a8 - changed drupalci to test only functional tests in update module
- 970cb189 - corrected testgroups in drupalci file
- 041eec54 - reverted changes in the drupalci file
- 3e15e4d8 - changed the error message according to the conditions
- bfeb3c78 - phpcbf
- b509db38 - reverted drupalci.yml
- 9609d239 - added condition for checking also available version
- 0bfa530b - added test coverage for the also available case
- 23d790c4 - phpcbf
- 56f6d8bf - changed order of release in fixture
- 61d65918 - Revert "changed order of release in fixture"
Toggle commit list-
a5a00051...81690f84 - 12 commits from branch
430 430 $this->standardTests(); 431 431 $this->confirmUnsupportedStatus('8.0.3', '8.1.0', 'Recommended version:'); 432 432 } 433 // Test when the newest branch is unsupported and no update is available. 434 foreach (['8.1.0', '8.1.0-beta1'] as $installed) { 435 $this->setProjectInstalledVersion($installed); 436 $this->refreshUpdateStatus([$this->updateProject => '1.1-unsupported']); 437 $this->standardTests(); 438 $this->confirmUnsupportedStatus($installed); 439 } 440 // Test when the current version is unsupported and the recommended version is supported and there is also an available version 441 $this->setProjectInstalledVersion('8.0.3'); 442 $this->refreshUpdateStatus([$this->updateProject => '1.0-supported']); 443 $this->standardTests(); 444 $this->confirmUnsupportedStatus('8.0.3', '8.1.0', 'Recommended version:'); 445 $this->confirmUnsupportedStatus('8.0.3', '8.2.0', 'Also available:'); -
I may have suggested this but it was because I forgot that the update module only uses "Also available:" for next majors not next minors. It will always recommend the last secure & supported version in the major as "Recommended version:". So we probably should remove the
8.2.0release fromsemver_test.1.0-supported.xmland the drupal version of the XML which is missing currently.This is going a problem for the Automatic Updates module because in this test case the MVP of the Automatic Updates module will not support jumping a minor.
changed this line in version 13 of the diff
added 20 commits
- d06a9d08 - added test coverage to prove that this problem currently exists
- 0171113b - added comment for the new test coverage
- 408d7b3d - phpcbf
- ead2e8ce - added new variable to determine the release is supported
- c45fdf23 - changed drupalci to test only functional tests in update module
- d831c763 - corrected testgroups in drupalci file
- 8948b513 - Merge branch '9.3.x' into 3227518-never-show-a
- b151c1a2 - reverted changes in the drupalci file
- 298cc5a6 - changed the error message according to the conditions
- a9de5ab2 - phpcbf
- 8930855f - reverted drupalci.yml
- 9ea36272 - added condition for checking also available version
- 39960219 - Merge branch '9.3.x' into 3227518-never-show-a
- 55cd579f - added test coverage for the also available case
- be754446 - phpcbf
- 33f1a9f1 - changed order of release in fixture
- 04c37ada - Merge branch '9.3.x' into 3227518-never-show-a
- e9d67d0e - merge conflict resolved
- ef4605b9 - Merge branch '3227518-never-show-a' of git.drupal.org:issue/drupal-3227518...
- 98d94076 - added test to show the current logic is broken
Toggle commit list431 431 $this->confirmUnsupportedStatus('8.0.3', '8.1.0', 'Recommended version:'); 432 432 } 433 // Test when the newest branch is unsupported and no update is available. 434 foreach (['8.1.0', '8.1.0-beta1'] as $installed) { 435 $this->setProjectInstalledVersion($installed); 436 $this->refreshUpdateStatus([$this->updateProject => '1.1-unsupported']); 437 $this->standardTests(); 438 $this->confirmUnsupportedStatus($installed); 439 } 440 $this->setProjectInstalledVersion('8.0.3'); 441 $this->refreshUpdateStatus([$this->updateProject => '1.0-supported']); 442 $this->standardTests(); 443 // This shows how the logic is currently broken as it skips all the releases 444 // between 8.0.3 and 10.0.0. 445 $this->confirmUnsupportedStatus('8.0.3', '10.0.0', 'Recommended version:'); 446 $this->assertUpdateTableTextNotContains('Also available'); - Comment on lines +445 to +446
This shows a related error if you’re in an unsupported branch then it will try to choose another target major but it will always choose the last one not the next one. The test xml fixture has 8.1.0, 9.0.0, 10.0.0 supported releases after 8.0.3 but it chooses the 10.0.0 as the recommended one. I will push up a change to fix this next.
changed this line in version 14 of the diff