Skip to content

[UI Test] - Add validation on new product screen and new add product tests #8407

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 15, 2022
Merged

[UI Test] - Add validation on new product screen and new add product tests #8407

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1118ad3
add validation on new product screen and new ui tests
jostnes Dec 14, 2022
c9a64bc
use equal instead of contains, rename for consistency
jostnes Dec 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions WooCommerce/Classes/ViewRelated/Products/Edit Product/ProductFormTableViewDataSource.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,7 @@ private extension ProductFormTableViewDataSource {
ratingCount: ratingCount,
averageRating: averageRating)
cell.accessoryType = .disclosureIndicator
cell.accessibilityIdentifier = "product-review-cell"
}

func configureSettingsRowWithASwitch(cell: UITableViewCell, viewModel: ProductFormSection.SettingsRow.SwitchableViewModel) {
Expand Down
3 changes: 2 additions & 1 deletion WooCommerce/UITestsFoundation/Screens/Products/ProductsScreen.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,8 @@ public final class ProductsScreen: ScreenObject {
}

public func selectProductType(productType: String) throws -> SingleProductScreen {
app.staticTexts[productType].tap()
let productTypeLabel = NSPredicate(format: "label CONTAINS[c] %@", productType)
app.staticTexts.containing(productTypeLabel).firstMatch.tap()
Comment on lines +85 to +86
Copy link
Contributor

@pachlava pachlava Dec 14, 2022

Choose a reason for hiding this comment

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

Just thinking aloud: the buttons for product type selection have no identifiers (as of now), so identifiers can't be used (which would be justified, since this non-asserting function can potentially be used in tests for localizations). So, we can go for exact text values (as you did before). Is there a reason for less strict locator (only part of a string / case-insensitive) that you're using now?

Copy link
Contributor Author

@jostnes jostnes Dec 15, 2022

Choose a reason for hiding this comment

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

yeah i actually tried to add identifiers for the cells but it looks like it will take more effort than i anticipated so i dropped that for now.

so the reason is that i wanted to be able to reuse it in the test like this:

            ...
            .selectProductType(productType: productType)
            .verifyProductTypeScreenLoaded(productType: productType)
            ...

the test wants the product type in both, using simple physical product as an example, in select product the text for that is Simple physical product and in add product screen it's Physical, the common denominator in both is the word physical (case-insensitive) so i decided to go with that. it fits the needs of the test case while still keeping it reusable and clear.

return try SingleProductScreen()
}
}
26 changes: 26 additions & 0 deletions WooCommerce/UITestsFoundation/Screens/Products/SingleProductScreen.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,4 +46,30 @@ public final class SingleProductScreen: ScreenObject {
XCTAssertTrue(app.staticTexts["TIP"].exists)
XCTAssertTrue(app.textViews[productName].exists)
}

public func verifyProductTypeScreenLoaded(productType: String) throws -> Self {
let priceLabel = NSPredicate(format: "label CONTAINS[c] %@", "price")
Copy link
Contributor

@pachlava pachlava Dec 14, 2022

Choose a reason for hiding this comment

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

WDYT of renaming priceLabel to addPriceLabel, as done for addVariationLabel?

Copy link
Contributor Author

@jostnes jostnes Dec 15, 2022

Choose a reason for hiding this comment

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

updated in c9a64bc

let inventoryLabel = NSPredicate(format: "label CONTAINS[c] %@", "inventory")
let productTypeLabel = NSPredicate(format: "label CONTAINS[c] %@", productType)
let addVariationLabel = NSPredicate(format: "label CONTAINS[c] %@", "add variations")
Copy link
Contributor

@pachlava pachlava Dec 14, 2022

Choose a reason for hiding this comment

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

Since this is an asserting function, I think it would benefit from more strict matching (this is a tested code):

Suggested change
let priceLabel = NSPredicate(format: "label CONTAINS[c] %@", "price")
let inventoryLabel = NSPredicate(format: "label CONTAINS[c] %@", "inventory")
let productTypeLabel = NSPredicate(format: "label CONTAINS[c] %@", productType)
let addVariationLabel = NSPredicate(format: "label CONTAINS[c] %@", "add variations")
let priceLabel = NSPredicate(format: "label == 'Add Price'")
let inventoryLabel = NSPredicate(format: "label == 'Inventory'")
let productTypeLabel = NSPredicate(format: "label ==[c] '\(productType)'")
let addVariationLabel = NSPredicate(format: "label == 'Add variations'")

Also, it would be easier to understand what exactly is missing, if an assertion fails.

Copy link
Contributor Author

@jostnes jostnes Dec 15, 2022

Choose a reason for hiding this comment

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

updated in c9a64bc


// the common fields on add product screen
XCTAssertTrue(app.cells["product-review-cell"].exists)
XCTAssertTrue(app.staticTexts.containing(productTypeLabel).firstMatch.exists)

// different product types displays different fields on add product screen
// this is to validate that the correct screens are displayed
switch productType {
case "physical", "virtual":
XCTAssertTrue(app.staticTexts.containing(priceLabel).firstMatch.exists)
XCTAssertTrue(app.staticTexts.containing(inventoryLabel).firstMatch.exists)
case "variable":
XCTAssertTrue(app.staticTexts.containing(addVariationLabel).firstMatch.exists)
XCTAssertTrue(app.staticTexts.containing(inventoryLabel).firstMatch.exists)
Copy link
Contributor

@pachlava pachlava Dec 14, 2022

Choose a reason for hiding this comment

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

inventoryLabel is present for both cases, so it can be moved to common?

Suggested change
// different product types displays different fields on add product screen
// this is to validate that the correct screens are displayed
switch productType {
case "physical", "virtual":
XCTAssertTrue(app.staticTexts.containing(priceLabel).firstMatch.exists)
XCTAssertTrue(app.staticTexts.containing(inventoryLabel).firstMatch.exists)
case "variable":
XCTAssertTrue(app.staticTexts.containing(addVariationLabel).firstMatch.exists)
XCTAssertTrue(app.staticTexts.containing(inventoryLabel).firstMatch.exists)
XCTAssertTrue(app.staticTexts.containing(inventoryLabel).firstMatch.exists)
// different product types displays different fields on add product screen
// this is to validate that the correct screens are displayed
switch productType {
case "physical", "virtual":
XCTAssertTrue(app.staticTexts.containing(priceLabel).firstMatch.exists)
case "variable":
XCTAssertTrue(app.staticTexts.containing(addVariationLabel).firstMatch.exists)

Copy link
Contributor Author

@jostnes jostnes Dec 15, 2022

Choose a reason for hiding this comment

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

ah ok, this was written with the future in mind, the next product type (grouped) we introduce, inventoryLabel will no longer be a common field so i'll leave it as is for now.

Copy link
Contributor

@pachlava pachlava Dec 15, 2022

Choose a reason for hiding this comment

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

Thanks! This is what I thought about, just wanted to be sure 🙂

default:
// fail test if product type doesn't exist
Copy link
Contributor

@pachlava pachlava Dec 14, 2022

Choose a reason for hiding this comment

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

I think the line below this comment has a good descriptive error message, so it's not necessary:

Suggested change
// fail test if product type doesn't exist

Copy link
Contributor Author

@jostnes jostnes Dec 15, 2022

Choose a reason for hiding this comment

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

removed in c9a64bc

fatalError("Product Type \(productType) doesn't exist!")
}
return self
}
}
4 changes: 4 additions & 0 deletions WooCommerce/WooCommerce.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1129,6 +1129,7 @@
7E7C5F872719A93C00315B61 /* ProductCategoryListViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7E7C5F812719A93C00315B61 /* ProductCategoryListViewController.swift */; };
7E7C5F8B2719AEDA00315B61 /* EditProductCategoryListViewModelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7E7C5F8A2719AEDA00315B61 /* EditProductCategoryListViewModelTests.swift */; };
7E7C5F8F2719BA7300315B61 /* ProductCategoryCellViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7E7C5F8E2719BA7300315B61 /* ProductCategoryCellViewModel.swift */; };
80089F182949B92D0078C671 /* ProductFlow.swift in Sources */ = {isa = PBXBuildFile; fileRef = 80089F172949B92D0078C671 /* ProductFlow.swift */; };
800A5B58275483D6009DE2CD /* OrdersTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 800A5B57275483D6009DE2CD /* OrdersTests.swift */; };
800A5B5F27548F31009DE2CD /* site_info_wordpress_com.json in Resources */ = {isa = PBXBuildFile; fileRef = 800A5B5E27548F31009DE2CD /* site_info_wordpress_com.json */; };
800A5B6127548F53009DE2CD /* sites_posts_password.json in Resources */ = {isa = PBXBuildFile; fileRef = 800A5B6027548F53009DE2CD /* sites_posts_password.json */; };
Expand Down Expand Up @@ -3136,6 +3137,7 @@
7E7C5F812719A93C00315B61 /* ProductCategoryListViewController.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ProductCategoryListViewController.swift; sourceTree = "<group>"; };
7E7C5F8A2719AEDA00315B61 /* EditProductCategoryListViewModelTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = EditProductCategoryListViewModelTests.swift; sourceTree = "<group>"; };
7E7C5F8E2719BA7300315B61 /* ProductCategoryCellViewModel.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ProductCategoryCellViewModel.swift; sourceTree = "<group>"; };
80089F172949B92D0078C671 /* ProductFlow.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ProductFlow.swift; sourceTree = "<group>"; };
800A5B57275483D6009DE2CD /* OrdersTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OrdersTests.swift; sourceTree = "<group>"; };
800A5B5E27548F31009DE2CD /* site_info_wordpress_com.json */ = {isa = PBXFileReference; lastKnownFileType = text.json; path = site_info_wordpress_com.json; sourceTree = "<group>"; };
800A5B6027548F53009DE2CD /* sites_posts_password.json */ = {isa = PBXFileReference; lastKnownFileType = text.json; path = sites_posts_password.json; sourceTree = "<group>"; };
Expand Down Expand Up @@ -6780,6 +6782,7 @@
children = (
80E6FC742763579D0086CD67 /* MockDataReader.swift */,
800A5B9D275623FC009DE2CD /* LoginFlow.swift */,
80089F172949B92D0078C671 /* ProductFlow.swift */,
);
path = Flows;
sourceTree = "<group>";
Expand Down Expand Up @@ -11522,6 +11525,7 @@
80C3626F277453E8005CEAD3 /* ReviewsTests.swift in Sources */,
80E6FC79276C3FD60086CD67 /* MockDataReader.swift in Sources */,
CCDC49CD23FFFFF4003166BA /* LoginTests.swift in Sources */,
80089F182949B92D0078C671 /* ProductFlow.swift in Sources */,
800A5B58275483D6009DE2CD /* OrdersTests.swift in Sources */,
80AD2CA427858BAB00A63DE8 /* StatsTests.swift in Sources */,
800A5BCB2759CE4B009DE2CD /* ProductsTests.swift in Sources */,
Expand Down
17 changes: 17 additions & 0 deletions WooCommerce/WooCommerceUITests/Flows/ProductFlow.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import UITestsFoundation
import XCTest

class ProductFlow {

static func addAndVerifyNewProduct(productType: String) throws {
let product = try GetMocks.readNewProductData()

try TabNavComponent().goToProductsScreen()
.tapAddProduct()
.selectProductType(productType: productType)
.verifyProductTypeScreenLoaded(productType: productType)
.addProductTitle(productTitle: product.name)
.publishProduct()
.verifyNewProductScreenLoaded(productName: product.name)
}
}
4 changes: 2 additions & 2 deletions WooCommerce/WooCommerceUITests/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ The following flows are covered/planned to be covered by UI tests. Tests that ar
4. Products
- [x] Products list and single product screens load
- [x] Add new product - Simple physical product
- [ ] Add new product - Simple virtual product
- [ ] Add new product - Variable product
- [x] Add new product - Simple virtual product
- [x] Add new product - Variable product
- [ ] Add new product - Grouped product
- [ ] Add new product - External product
- [ ] Search for product
Expand Down
23 changes: 8 additions & 15 deletions WooCommerce/WooCommerceUITests/Tests/ProductsTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,6 @@ import XCTest

final class ProductsTests: XCTestCase {

let productTypes = [
"physical": "Simple physical product",
"virtual": "Simple virtual product",
"variable": "Variable product",
"grouped": "Grouped product",
"external": "External product"
]

override func setUpWithError() throws {
continueAfterFailure = false

Expand All @@ -33,13 +25,14 @@ final class ProductsTests: XCTestCase {
}

func test_add_simple_physical_product() throws {
let product = try GetMocks.readNewProductData()
try ProductFlow.addAndVerifyNewProduct(productType: "physical")
}

try TabNavComponent().goToProductsScreen()
.tapAddProduct()
.selectProductType(productType: productTypes["physical"]!)
.addProductTitle(productTitle: product.name)
.publishProduct()
.verifyNewProductScreenLoaded(productName: product.name)
func test_add_simple_virtual_product() throws {
try ProductFlow.addAndVerifyNewProduct(productType: "virtual")
}

func test_add_variable_product() throws {
try ProductFlow.addAndVerifyNewProduct(productType: "variable")
}
Comment on lines +31 to 37
Copy link
Contributor

@pachlava pachlava Dec 14, 2022

Choose a reason for hiding this comment

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

➕ for extracting the repetitive part to a flow. Nice 👍

}