Skip to content

Conversation

@rachelmcr
Copy link
Contributor

Closes: #6508

Description/Changes

While checking test coverage for Order Creation, we found a couple order-related gaps in the Yosemite layer.

  • OrderFactory.deletedShippingLine wasn't exercised in testing. This is because we weren't using it to remove the shipping line from an order. ShippingInputTransformer now uses it to update the order when a shipping line is removed, and it is exercised in test_new_input_deletes_shipping_line_from_order() in ShippingInputTransformerTests.
  • OrderNoteAction.addOrderNote had no test coverage. This isn't used in Order Creation, but it's worth adding coverage since the gap was identified. There are now tests in OrderNoteStoreTests to confirm that the expected note is returned and persisted, and that network errors are handled.

Testing

Confirm tests pass in CI.

You can also test the shipping line change as follows:

  1. Go to the Orders tab and create a new order.
  2. Select "Add Shipping", add a shipping amount, and tap Done.
  3. On the New Order screen, once the order syncs remotely, tap "Shipping."
  4. Select "Remove Shipping from Order."
  5. Tap "Create" and confirm the order is created without any shipping.

Submitter Checklist

Update release notes:

  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@rachelmcr rachelmcr added category: unit tests Related to unit testing. feature: order creation All tasks related to creating an order labels Apr 1, 2022
@rachelmcr rachelmcr added this to the 8.9 milestone Apr 1, 2022
@rachelmcr rachelmcr requested review from Ecarrion and ealeksandrov and removed request for Ecarrion April 1, 2022 16:54
Copy link
Contributor

@ealeksandrov ealeksandrov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for noticing and fixing the gaps in tests! LGTM :shipit:

(Data?, Error?) manipulation instead of Result feels ancient now :)

@rachelmcr rachelmcr merged commit 0118cee into trunk Apr 4, 2022
@rachelmcr rachelmcr deleted the issue/6508-order-creation-yosemite-tests branch April 4, 2022 10:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category: unit tests Related to unit testing. feature: order creation All tasks related to creating an order

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Order Creation: Add test coverage for gaps in the Yosemite layer

3 participants