Fix tests

Trying to fix.

Looks like some of the issues are from upstream commerce now. We'll have to check this more in deep.

@kul.pratap please use a separate MR to not break anything already working...

We need to get the pipeline green again, so other issues can be fixed with more confidence!
Setting priority to major for that reason.

Working on it.

@lavanyatalwar adding translations to tests is totally wrong (in most places), please revert these commits.
If you're unsure about changes you make, please use a separate branch for these changes to not pollute existing valid fixes!

Thank you.

Hi @anybody,

I apologize for the confusion with the recent changes. It looks like I misunderstood the requirements and implemented them incorrectly. I'll go ahead and revert the changes right away.

@rhovland: We should tackle this next together with 📌 Add Explicit Return Types and Discussion on Return Value Active

This last commit to fix getComment() brings the phpcs error "Only string literals should be passed to t() where possible"

From what I can understand passing variables to t() is wrong and should not be done. The source of the comments is where translation is supposed to be happening. Only done in the event subscriber.

Remaining errors:
GiftcardAdminTest::testRefund The text "Refund gift card" was not found anywhere in the text of the current page.

Best I can tell "Refund gift card" is not on the page after placing an order because the current user in the test doesn't have permissions for that route.

GiftcardRedemptionPaneTest:

All of these errors don't make sense because they should be present. The output of these tests aren't in the artifacts so I can't tell what is wrong here.

Ha I actually fixed the testRefund test...

Looking at the tests in commerce I noticed absolutely none of them clicked on order transition links/buttons and instead triggered a state change in the order with code then saved it. Clicking the place order button wasn't changing the order's state from draft. Additionally for whatever reason the refund gift card action link wasn't appearing on the resulting page.

Since the important parts of the first section of the test are verifying that the order has been placed (in completed state and gift card balance changed) and that the refund route is accessible I switched to invoking those directly. These should be functionally the same for the purposes of the test. Once on the refund page the rest of the test can proceed normally.

Working on moving the needed changes over to the merge request branch

Great work @rhovland!! Feel free to set this needs review, once it's ready and I'll ask our team to have a look :)

Ok I figured out what was causing the last group of totals errors. The creation of the tax is not working anymore in the setUp() function for the GiftcardRedemptionPaneTest. So the expected totals are off because the order has no tax on it.

Digging into the code of this test I'm starting to doubt it ever worked. It was literally in the wrong namespace (commerce_promotion) until the most recent changes to fix phpcs violations and such. Other than having taxes shoved in there there's a ton of similarities to commerce_promotion's CouponRedemptionPaneTest.

The problem as it stands is that I don't know how this custom made up tax is supposed to work so I can't begin to tell if the assert statements are testing it correctly and there's an issue with the tax setup or if the assertions are wrong and tax setup is correct.

I'll keep digging but I thought I'd share what I've found so far.

Thank you so much @rhovland! Great findings and progress. Sorry for the quietness on my end, I'll try to help asap again.

Fixed tests.
I removed the taxes from the test because they were breaking things and as implemented wasn't actually testing the interaction between taxes and giftcards. I opened a new issue to add a separate test for taxes. 📌 Add a test for tax interaction Active

Form submissions (review step, complete step) were broken because of upstream commerce changes. I used the CouponRedemptionPaneTest in upstream as a basis for changes to the test setup. This fixed form submissions.

There are 3 remaining test failures. They are legitimate failures caused by faulty code in commerce_giftcard.module. Specifically commerce_giftcard_preprocess_commerce_checkout_completion_message assumes that order items have purchased entities attached to them. This is optional for order items and will cause a WSOD on the checkout complete step of any stores creating basic order items on orders with no purchased entities attached (probably not common).
Additionally, the code wouldn't ever work anyways because it assumes that giftcard transactions have a reference to the order item id instead of the order id which is what is actually stored on transactions.

I will work to fix this bug tomorrow and button up the tests with some additional cleanup.

Created a separate branch with the code changes from 🐛 WSOD on order completion page when order has items without purchased entites Active .

All tests now pass once both issues are committed.

Doing some code cleanup on tests and then this is ready for review.

Code cleanup done. This is ready for review and commit and so is 🐛 WSOD on order completion page when order has items without purchased entites Active

Impressive work @rhovland! :) I'll ask @grevil for a review in the next days. Thank you very much!

Test finished for 🐛 WSOD on order completion page when order has items without purchased entites Active

Thanks, @rhovland!

I'll come back to you asap, putting this issue and 🐛 WSOD on order completion page when order has items without purchased entites Active on my list!