Problem/Motivation

Semantic versions for Drupal 8 were introduced in #2170443: [meta] Create a plan for implementing semantic versioning for core. Update status still tests update XML data with Drupal 7 major versions. There is no apparent test coverage for this. Given that this is very important part of our security infrastructure, it would be very important to get this right.

Proposed resolution

Add tests for when versions are released:

  • 8.0.0
  • 8.0.1
  • 8.1.0-alpha5
  • 8.1.0-beta3
  • 8.1.0
  • 9.0.0

Fix any issues found.

Remaining tasks

Do it.

User interface changes

None.

API changes

Likely none.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

drumm’s picture

Issue summary: View changes

Update status XML samples for future core versions can be generated by doing the release process on git7staging. It would be best to have a list of versions that are needed for test coverage, so they can be done all at once. I started a draft list under Proposed resolution.

There is a chance that update status should have different messaging for 8.1.0 being available vs. 8.0.1, they will be different enough upgrades. And we should check the messaging when both are available as upgrades.

Gábor Hojtsy’s picture

Modifiers would be good to add, eg. 8.2.0-alpha1, what happens when you have that vs. 8.1.3 (also available) when you are on 8.1.2? :)

Also contrib versions would also be interesting, eg. Panels 8.2.x-3.1 to Panels 8.2.x-5.6 and/or Panels 8.3.x-1.2 or somesuch. If we think covering contrib here is too much of a pain, we should open yet another critical for that IMHO.

drumm’s picture

drumm’s picture

I should say contrib is sticking to version numbers like 8.x-1.2.

Gábor Hojtsy’s picture

@drumm: given that 8.1, 8.2, etc. may have non-backwards compatible API changes, I'm not sure that can fly at all. Posted more about that at the issue you suggested: #1612910-184: [policy, no patch] Switch to Semantic Versioning for Drupal contrib extensions (modules, themes, etc). Anyway, since that is totally undecided so far, looks like we should focus on core only here for now and maybe open a contrib issue later if this is otherwise done :)

Gábor Hojtsy’s picture

Issue summary: View changes
Issue tags: +Drupalaton 2014

Looks like they may not be that backwards incompatible anymore... So contrib will be generally compatible with 8.x. So ok, let's fix this for core and ignore contrib versioning for now. #1612910: [policy, no patch] Switch to Semantic Versioning for Drupal contrib extensions (modules, themes, etc) is not even a release blocker anymore.

I think the proposed numbers are good except I would add some extra suffixes as well to the mix to ensure they work. alpha and beta maybe :) Added those two the mix.

drumm’s picture

FileSize
269.06 KB

Attached are update status XML files as if each release has been created.

Devin Carlson’s picture

Status: Active » Needs review
FileSize
45.5 KB

Taking a first stab at this.

I didn't add semantic versioning tests for contrib modules/themes per #3-#6.

star-szr’s picture

Status: Needs review » Needs work

Thank you @Devin Carlson!

The existing patch looks good but doesn't contain any -alpha, -beta, or 9.x.x releases; those should probably be added per the issue summary.

I think any messaging/UI concerns would be for another issue.

Gábor Hojtsy’s picture

Agreed with @Cottser.

@Devin: note @drumm has some sample update XMLs above with alpha and beta stuff.

Devin Carlson’s picture

Assigned: Unassigned » Devin Carlson
Status: Needs work » Needs review
FileSize
45.53 KB

Thanks for the review and extra info Cottser and Gábor!

Working on adding additional coverage per #9. Attaching a reroll of #8 to verify that it passes after all of the changes made over the last couple of weeks (will set back to needs work).

effulgentsia’s picture

Status: Needs review » Needs work

#11 is green, yay. Back to needs work per the comment.

Devin Carlson’s picture

Status: Needs work » Needs review
FileSize
109.48 KB

An updated patch that adds coverage for -alpha, -beta and updating from Drupal 8.x.x-x to 9.0.0.

Gábor Hojtsy’s picture

Devin, can you post an interdiff between 11 and 13? That would help reviewing it immensely. Reviewing a 100k patch otherwise is not trivial :)

Devin Carlson’s picture

FileSize
80.78 KB

Interdiff between #11 and #13.

effulgentsia’s picture

#13 looks really good to me, especially all the extra cases covered by the test*Available() tests. But, I'm wondering why the following tests:

+++ b/core/modules/update/src/Tests/UpdateCoreTest.php
@@ -98,82 +186,112 @@ function testDatestampMismatch() {
   function testModulePageRunCron() {
   function testModulePageUpToDate() {
   function testModulePageRegularUpdate() {
   function testModulePageSecurityUpdate() {
   function testLanguageModuleUpdate() {

all require

-    $this->setSystemInfo7_0();
+    foreach (array(0, 1) as $minor_version) {
+      foreach (array('-alpha1', '-beta1', '') as $extra_version) {
+        $this->setSystemInfo("8.$minor_version.0" . $extra_version);

Looks to me like they're testing stuff that's independent of starting version, so I don't see what we gain by iterating all the versions. Would it make sense for these tests to just test an 8.0.0 starting point?

Also, same question for UpdateUploadTest.

Devin Carlson’s picture

Addressing the feedback in #16.

effulgentsia’s picture

Status: Needs review » Reviewed & tested by the community

Thanks!

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed cac5f7a and pushed to 8.0.x. Thanks!

  • alexpott committed cac5f7a on 8.0.x
    Issue #2315849 by Devin Carlson | Gábor Hojtsy: Update status does not...

Status: Fixed » Closed (fixed)

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