Problem/Motivation

Head tests are failing.

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#25 interdiff.txt2.16 KBquietone
#25 2906650-25.patch7.46 KBquietone
#24 interdiff.txt1.14 KBquietone
#24 2906650-24.patch5.12 KBquietone
#22 interdiff.txt7.26 KBquietone
#22 2906650-22.patch3.84 KBquietone
#13 interdiff_11-13.txt229 bytesheddn
#13 2906650-13.patch4.99 KBheddn
#11 interdiff_10-11.txt429 bytesheddn
#11 2906650-11.patch5.04 KBheddn
#10 2906650-10.patch4.95 KBheddn
#10 interdiff_8-10.txt542 bytesheddn
#8 interdiff_2-8.txt10.38 KBheddn
#8 2906650-8.patch5.08 KBheddn
#7 Screenshot_2017-09-05_07-51-48.png7.66 KBheddn
#5 Screenshot_2017-09-04_16-52-45.png29.31 KBheddn
#2 2906650.patch13.54 KBheddn
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

heddn created an issue. See original summary.

heddn’s picture

heddn
Primary language English
Location Nicaragua
CreditAttribution: heddn at MTech, LLC, Acro Commerce commented
FileSize
2906650.patch13.54 KB
heddn’s picture

heddn
Primary language English
Location Nicaragua
CreditAttribution: heddn at MTech, LLC, Acro Commerce commented

I've tried reproducing the error on local with commerce commit: 75ce6a3841846b7637e9582bd6e02b6492504794 and I'm not able to do that (yet).

Status: Needs review » Needs work

The last submitted patch, 2: 2906650.patch, failed testing. View results

heddn’s picture

heddn
Primary language English
Location Nicaragua
CreditAttribution: heddn at MTech, LLC, Acro Commerce commented
FileSize
Screenshot_2017-09-04_16-52-45.png29.31 KB

I found some interesting other bugs: #2906684: ck2 dump is malformed and #2906683: default store is not migrated. And I'm able to confirm that the data does seem to be migrated correctly. But I'm still not sure what is the deal with the tests and I'm not able to reproduce the issue locally.

heddn’s picture

heddn
Primary language English
Location Nicaragua
CreditAttribution: heddn at MTech, LLC, Acro Commerce commented

Also found #2906663: ProductDisplay and Order store resolver is fragile while at all this too.

heddn’s picture

heddn
Primary language English
Location Nicaragua
CreditAttribution: heddn at MTech, LLC, Acro Commerce commented
Issue summary: View changes
FileSize
Screenshot_2017-09-05_07-51-48.png7.66 KB

And here's a completed order, displaying 'Hat 2' instead of the SKU. I wonder if the version of commerce being downloaded isn't the dev branch. Going to check on composer and .info.yml dependencies next.

heddn’s picture

heddn
Primary language English
Location Nicaragua
CreditAttribution: heddn at MTech, LLC, Acro Commerce commented
Status: Needs work » Needs review
FileSize
2906650-8.patch5.08 KB
interdiff_2-8.txt10.38 KB

Status: Needs review » Needs work

The last submitted patch, 8: 2906650-8.patch, failed testing. View results

heddn’s picture

heddn
Primary language English
Location Nicaragua
CreditAttribution: heddn at MTech, LLC, Acro Commerce commented
Status: Needs work » Needs review
FileSize
interdiff_8-10.txt542 bytes
2906650-10.patch4.95 KB
heddn’s picture

heddn
Primary language English
Location Nicaragua
CreditAttribution: heddn at MTech, LLC, Acro Commerce commented
FileSize
2906650-11.patch5.04 KB
interdiff_10-11.txt429 bytes

Status: Needs review » Needs work

The last submitted patch, 11: 2906650-11.patch, failed testing. View results

heddn’s picture

heddn
Primary language English
Location Nicaragua
CreditAttribution: heddn at MTech, LLC, Acro Commerce commented
Status: Needs work » Needs review
FileSize
2906650-13.patch4.99 KB
interdiff_11-13.txt229 bytes

The last submitted patch, 10: 2906650-10.patch, failed testing. View results

Status: Needs review » Needs work

The last submitted patch, 13: 2906650-13.patch, failed testing. View results

  • heddn committed 2b28e59 on 8.x-2.x 
    Issue #2906650 by heddn: Test failures on HEAD: Drupal\Tests\...
heddn’s picture

heddn
Primary language English
Location Nicaragua
CreditAttribution: heddn at MTech, LLC, Acro Commerce commented
Status: Needs work » Needs review

Commited composer and .info.yml changes so testbot would pick them up. Changes to those files aren't picked up by the bot and I think that might be part of our problem. Fingers crossed.

  • heddn committed 912284c on 8.x-2.x 
    Issue #2906650 by heddn: Test failures on HEAD: Drupal\Tests\...

  • heddn committed 5007373 on 8.x-2.x 
    Issue #2906650 by heddn, mglaman: Test failures on HEAD: Drupal\Tests\...

heddn credited mglaman.

heddn’s picture

heddn
Primary language English
Location Nicaragua
CreditAttribution: heddn at MTech, LLC, Acro Commerce commented
Status: Needs review » Needs work

Conferred with @mglaman and we're going to just disable the failing testing until we have more time to figure this out.

quietone’s picture

quietone CreditAttribution: quietone at Acro Commerce commented
Status: Needs work » Needs review
FileSize
2906650-22.patch3.84 KB
interdiff.txt7.26 KB

The line_item_label in commerce_line_item is the SKU. Therefor, modified the source plugin to get the title from the commerce_product table.

Status: Needs review » Needs work

The last submitted patch, 22: 2906650-22.patch, failed testing. View results

quietone’s picture

quietone CreditAttribution: quietone at Acro Commerce commented
Status: Needs work » Needs review
FileSize
2906650-24.patch5.12 KB
interdiff.txt1.14 KB

And update the other order test to use the correct title/label and not the SKU.

quietone’s picture

quietone CreditAttribution: quietone at Acro Commerce commented
FileSize
2906650-25.patch7.46 KB
interdiff.txt2.16 KB

Reviewing the issues, I see that this is the same as #2900785: Order item migration and purchasable entity labels. Which I didn't realize before I wrote this patch, so I did the same code twice. Lucky me. The other patch has a source plugin test, which is being added now.

heddn’s picture

heddn
Primary language English
Location Nicaragua
CreditAttribution: heddn at MTech, LLC, Acro Commerce commented
Status: Needs review » Reviewed & tested by the community

Looks good now. Tests are passing and the explanation for why the failures makes sense.

  • heddn committed fd616d3 on 8.x-2.x authored by quietone 
    Issue #2906650 by heddn, quietone, mglaman: Test failures on HEAD:...
heddn’s picture

heddn
Primary language English
Location Nicaragua
CreditAttribution: heddn at MTech, LLC, Acro Commerce commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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