Skip to content
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.

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

Test automation starter #44

Merged
merged 24 commits into from Jul 11, 2018
Merged

Test automation starter #44

merged 24 commits into from Jul 11, 2018

Conversation

dkaravias
Copy link
Contributor

Adding the whole app to master.
Worked with Sven.
The next PRs will be smaller and will reflect the work done for the Test automation guidelines

Copy link

@bartzet bartzet left a comment

Choose a reason for hiding this comment

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

Let's clean up those empty lines to be more consistent. Not only in the files I marked but it would be good to go through all of them again.


## Getting started

Before launching the app you'll need to start the server. Make sure you have Ruby installed by running `ruby -v` from the terminal. If it's not installed run `brew install ruby`
Copy link

Choose a reason for hiding this comment

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

Shall we also add the information on how to start the server or at least point to the website how to do that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's the next line, where it says Navigate to the simplesinatraapi directory in this project and run ruby server.rb . The directory is included in the project.

fun verifyHeader() {
HEADER.check(matches(isDisplayed()))
}

Copy link

Choose a reason for hiding this comment

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

Empty line


import com.novoda.androidstoreexample.dagger.module.HostModule


Copy link

Choose a reason for hiding this comment

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

Empty line

@dkaravias
Copy link
Contributor Author

@bartzet I formatted the whole project with https://github.com/shyiko/ktlint
All Kotlin files should be consistent now.

@fourlastor fourlastor self-requested a review July 6, 2018 15:06
fun withCategoryTitle(title: String): Matcher<RecyclerView.ViewHolder> {
return object : BoundedMatcher<RecyclerView.ViewHolder, CategoryAdapter.Holder>(CategoryAdapter.Holder::class.java) {
override fun matchesSafely(item: CategoryAdapter.Holder): Boolean {
if (item.categoryName != null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

you can do return item.categoryName?.let { it.text.toString().equals(title, true) } ?: false

}

override fun describeTo(description: Description) {
description.appendText("view holder with title: " + title)
Copy link
Contributor

Choose a reason for hiding this comment

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

you can do "with title: $title"

}

override fun matchesSafely(item: ProductListAdapter.Holder?): Boolean {
if (item?.productName != null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

you can use the let here (as before)

fun navigateToProductList() {
val categoryMatcher = ViewMatchers.withCategoryTitle("HATS")

onView(withId(R.id.categoryListView)).perform(actionOnHolderItem(categoryMatcher, click()))
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd split this in 2 lines

import static android.support.test.espresso.matcher.ViewMatchers.isDisplayed;
import static android.support.test.espresso.matcher.ViewMatchers.withId;

@RunWith(AndroidJUnit4.class)
Copy link
Contributor

Choose a reason for hiding this comment

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

this isn't required anymore btw (doesn't do anything)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice, tbh the whole test is redundant by now but we can leave it to have a Java/Kotlin side by side example (also demonstrating the difference between using or not using page objects)

import java.util.*

data class CategoryResponse(val categories: Array<Category>) {
override fun equals(other: Any?): Boolean {
Copy link
Contributor

Choose a reason for hiding this comment

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

this is a data class, equals and hashCode are already implemented, unless you need a very specific behavior, I recommend removing both

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd rather check with @thywen who wrote this

import java.util.*

data class ProductResponse(val products: Array<Product>) {
override fun equals(other: Any?): Boolean {
Copy link
Contributor

Choose a reason for hiding this comment

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

same here about equals/hashCode

Copy link
Contributor Author

Choose a reason for hiding this comment

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

same as above


def add_to_basket(item)
item_id = item.id
p item_id
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the print can be removed

def initialize
@items = { '3': [
Item.new(1, 'Hat Green', '$18', 'hat1', get_random_description(4)),
Item.new(2, 'Hat Black', '$12', 'hat2', get_random_description),
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd remove the default and pass 2 here, it's used as a default only once and it makes this look weird 😄

Faker::Hipster.sentences(number_of_sentences).join(' ')
end

def get_item_for_id(id)
Copy link
Contributor

Choose a reason for hiding this comment

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

this looks more like a find_first_item_in_category(category_id)

also, if the category has zero items, you'll get an index out of bounds

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Again, care to explain when you have some time?


override fun loadCategoryList(categoryListListener: CategoryListListener) {
call = apiService.getCategories()
call.enqueue(object : Callback<CategoryResponse> {
call?.enqueue(object : Callback<CategoryResponse> {
override fun onResponse(call: Call<CategoryResponse>?, response: Response<CategoryResponse>?) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you can remove the nullables also here


override fun loadProductList(produListListener: ProductListListener, category: Int) {
call = apiService.getProductsFromCategory(category)
call.enqueue(object : Callback<ProductResponse> {
call?.enqueue(object : Callback<ProductResponse> {
override fun onFailure(call: Call<ProductResponse>?, t: Throwable?) {
Copy link
Contributor

Choose a reason for hiding this comment

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

and here (removing the nullables)

}

override fun onItemClicked(type: Int) {
intent = Intent(this, ProductListActivity::class.java).apply { putExtra(CATEGORY_ID_EXTRA, type) }
Copy link
Contributor

Choose a reason for hiding this comment

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

this is setting the activity intent ⚠️ be sure you use val

@dkaravias dkaravias added the PT label Jul 10, 2018
@jgtaylor123
Copy link
Contributor

Gave it a walk through, tests are running. Looks good.

Copy link
Contributor

@jgtaylor123 jgtaylor123 left a comment

Choose a reason for hiding this comment

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

👍 👍

@jgtaylor123 jgtaylor123 merged commit e5c0687 into master Jul 11, 2018
@dkaravias dkaravias deleted the test_automation_starter branch December 12, 2018 13:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants