[UI Test] - Add simple physical product UI Test #8380
Conversation
jostnes
added
feature: add/edit products
You can test the changes from this Pull Request by:
|
pachlava
left a comment
pachlava
left a comment
There was a problem hiding this comment.
Hi @jostnes 🙂 👋
Thanks for the new test! I especially like the parts related to adding new accessibility identifiers 👍.
There's a typo in the products types list for the virtual product, this is the only reason why I'm opting for "Comment"-type review, and not approving.
I also left some other notes - feel free to consider them.
| app.staticTexts[productType].tap() | ||
|
|
||
| return try SingleProductScreen() |
There was a problem hiding this comment.
Just noticing that there's a line break before return, while in two functions above there isn't:
| app.staticTexts[productType].tap() | |
| return try SingleProductScreen() | |
| app.staticTexts[productType].tap() | |
| return try SingleProductScreen() |
| app.cells["product-title"].tap() | ||
| app.cells["product-title"].typeText(productTitle) |
There was a problem hiding this comment.
Tap followed by entering text is present in clearAndEnterText from XCTest+Extensions:
| app.cells["product-title"].tap() | |
| app.cells["product-title"].typeText(productTitle) | |
| app.cells["product-title"].clearAndEnterText(text: productTitle) |
The downside of it is that it also tries to clear existing text before entering, which takes time, so I'd recommend extracting the tap+type lines into a separate XCUIElement.enterText and call it instead.
There was a problem hiding this comment.
the initial version of this was using clearAndEnterText() but clearing the text field isn't needed in this case so i changed it. good idea on extracting only tap and type, i'll do that! thanks for the suggestion!
| XCTAssertTrue(app.buttons["save-product-button"].exists) | ||
| XCTAssertTrue(app.staticTexts["TIP"].exists) |
There was a problem hiding this comment.
WDYT of checking that this screen contains the product name?
There was a problem hiding this comment.
|
|
||
| let productTypes = [ | ||
| "physical": "Simple physical product", | ||
| "virtual": "Virtual physical product", |
There was a problem hiding this comment.
I think it should say
| "virtual": "Virtual physical product", | |
| "virtual": "Simple virtual product", |
There was a problem hiding this comment.
oof - such a bad typo, thanks for catching this! will fix it
There was a problem hiding this comment.
pachlava
left a comment
pachlava
left a comment
There was a problem hiding this comment.
Thanks for all the follow-ups and changes @jostnes! 🎉
I took the liberty of making a tiny commit 718db71, to save up on back and forth's, so that you can merge it once you're back online.
It now looks a bit questionable to approve a PR in which I made a commit, but I see no reasons why it can't be approved 😅.
Thank you once again! 
|
I think it's fine, since the change you made is a small change that I missed 😅 Thank you for the thorough review as always! 🚀 |
Description
This PR adds a new test case
Add new product - Simple physical productto the UI test suite. I've removed the first product currently on the Products list so that that product can be the new product. Also, I realized that the Create Order tests are dependent on this value on the mock file, so I've updated some of the orders mock with the new values.To keep the test light while still maintaining the test's purpose (to validate that adding a simple physical product works), only the product name is added from the UI, and other data are added to the mock file.
The validation is on the new product screen where the test validates the existence of the Save button and an announcement (both of which do not appear when selecting the product from the product list)
Testing instructions
The new test
test_add_simple_physical_product()should pass in CI and locally. The test should also fail when verification points are not met.RELEASE-NOTES.txtif necessary.