Tests from #1017672: D6 to D7 update process permanently deletes comment bodies and other data, and throws fatal SQL errors inexplicably started failing a week or so ago. This issue is for figuring out why and getting them applied.

Attaching just the test hunk here.

CommentFileSizeAuthor
#14 node-type-disabled-1160928-13.patch2.58 KBxjm
#14 interdiff.txt857 bytesxjm
#12 node_type_disabled_1160928.patch2.57 KBcatch
#10 node_type_disabled_1160928.patch1.6 KBcatch
just_tests.patch2.74 KBcatch
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented

Subscribing.

Status: Needs review » Needs work

The last submitted patch, just_tests.patch, failed testing.

Tor Arne Thune’s picture

Tor Arne Thune CreditAttribution: Tor Arne Thune commented
Status: Needs work » Needs review

just_tests.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, just_tests.patch, failed testing.

Tor Arne Thune’s picture

Tor Arne Thune CreditAttribution: Tor Arne Thune commented

Has the file modules/simpletest/tests/upgrade/upgrade.node.test been removed?

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented

@1V - it was removed from Drupal 8, but not Drupal 7.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Status: Needs work » Needs review

just_tests.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, just_tests.patch, failed testing.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Status: Needs work » Needs review
FileSize
node_type_disabled_1160928.patch1.6 KB

Doh, what about this then.

Status: Needs review » Needs work

The last submitted patch, node_type_disabled_1160928.patch, failed testing.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Status: Needs work » Needs review
FileSize
node_type_disabled_1160928.patch2.57 KB
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented

So two issues:

1. The upgrade database had disappeared in one of the many re-rolls on that issue.

2. The test class was doubled up in the file, likely patch applied over itself or something.

So no actual code changes here compared to the patches in #1017672: D6 to D7 update process permanently deletes comment bodies and other data, and throws fatal SQL errors, just fixing the patch itself.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Status: Needs review » Reviewed & tested by the community
FileSize
interdiff.txt857 bytes
node-type-disabled-1160928-13.patch2.58 KB

Compulsively sneaking in some doxygen correction with an RTBC.

catch pointed out that this was tested by itself in #1017672-16: D6 to D7 update process permanently deletes comment bodies and other data, and throws fatal SQL errors. (You can see the additional fail over the 29 for the combined patch there, and checking the test results shows the failed assertion: "Comment body field instance was created for comments attached to the disabled forum node type.")

The approach looks reasonable and the test is straightforward and simple.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

Great!

Committed and pushed to 7.x. Thanks!

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Title: Add tests for node type/comment body upgrade path » [D7 HEAD broken] Add tests for node type/comment body upgrade path
Status: Fixed » Reviewed & tested by the community

@webchick: You forgot to add the new file.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Title: [D7 HEAD broken] Add tests for node type/comment body upgrade path » Add tests for node type/comment body upgrade path
Status: Reviewed & tested by the community » Fixed

Duh. One of these days I will remember to do that. :P

Fixed.

Status: Fixed » Closed (fixed)

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