hatepoint / phrases-hyperskill-tests

0 stars 1 forks source link

incorrect asserting of list items #7

Closed RedJocker closed 1 year ago

RedJocker commented 1 year ago

this code seems to be an attempt to make assertions on list items, but this is incorrect

protected val recyclerViewItems: RecyclerView by lazy {
        val view = activity.findViewByString<RecyclerView>("recyclerView")
        assertNotNull("The recyclerView adapter should not be null", view)
        val phraseTV = view.findViewByString<TextView>("phraseTextView")
        val deleteTV = view.findViewByString<TextView>("deleteTextView")
        view
    }

A recyclerView has potentially many listItems, if you want to search views on listItems you need the itemView to do the search. You can use the assertListItems to make assertions on all list items. If you need a function to do assertion on just one specific item we can write a variation of assertListItems that does that.

other things

  1. you already have a property recyclerView, there is no need to do the findViewByString again.
  2. assertNotNull for the return of findViewByString is redundant
  3. this should be a fun not a val by lazy
hatepoint commented 1 year ago

Is this better?

protected fun assertRecyclerViewItems() {
        val view = recyclerView
        val phraseTV = view.findViewHolderForAdapterPosition(0)?.itemView?.findViewByString<TextView>("phraseTextView")
        val deleteTV = view.findViewHolderForAdapterPosition(0)?.itemView?.findViewByString<TextView>("phraseTextView")
    }
RedJocker commented 1 year ago

Yes. It can be a little better like this

protected fun assertRecyclerViewFirstItem() {
        val phraseTV = recyclerView.findViewHolderForAdapterPosition(0)?.itemView?.findViewByString<TextView>("phraseTextView")
        val deleteTV = recyclerView.findViewHolderForAdapterPosition(0)?.itemView?.findViewByString<TextView>("phraseTextView")
    }

but with my experience i think there might still be some problems with null values comming from findViewHolderForAdapterPosition(0).

This is the best I come up with on MusicPlayer project

/**
     *  Makes assertions on the contents of one item of the RecyclerView.
     *
     *  Asserts that the the size of the list is at least itemIndex + 1.
     *
     *  Calls assertItem with the itemViewSupplier so that it is possible to make assertions on that itemView.
     *  Take attention to refresh references to views coming from itemView since RecyclerView
     *  can change the instance of View for a determinate list item after an update to the list.
     */
    fun RecyclerView.assertSingleListItem(itemIndex: Int, assertItem: (itemViewSupplier: () -> View) -> Unit) {

        assertNotNull("Your recycler view adapter should not be null", this.adapter)

        val expectedMinSize = itemIndex + 1

        val actualSize = this.adapter!!.itemCount
        assertTrue(
            "RecyclerView was expected to contain item with index $itemIndex, but its size was $actualSize",
            actualSize >= expectedMinSize
        )

        if(actualSize >= expectedMinSize) {
            val firstItemViewHolder = (0 until actualSize)
                .asSequence()
                .mapNotNull {  this.findViewHolderForAdapterPosition(it) }
                .firstOrNull()
                ?: throw AssertionError("No item is being displayed on songList RecyclerView, is it big enough to display one item?")

            val listHeight = firstItemViewHolder.itemView.height * (expectedMinSize + 1)
            this.layout(0,0, this.width, listHeight)  // may increase clock time

            val itemViewSupplier = {
                this.scrollToPosition(itemIndex)
                val itemView = (this.findViewHolderForAdapterPosition(itemIndex)?.itemView
                    ?: throw AssertionError("Could not find list item with index $itemIndex"))
                itemView

            }

            assertItem(itemViewSupplier)

        } else {
            throw IllegalStateException("size assertion was not effective")
        }
    }

to use like this


    recyclerView.assertSingleListItem(index) { itemViewSupplier ->
        var itemView = itemViewSupplier()
        var phraseTV = itemView.findViewByString<TextView>("phraseTextView")
        var deleteTV = itemView.findViewByString<TextView>("phraseTextView")

        // ... trigger events and do assertions

        // if a event triggered changes items on recycler view refresh the itemView reference and child views too
        itemView = itemViewSupplier()
        phraseTV = itemView.findViewByString<TextView>("phraseTextView")
        deleteTV = itemView.findViewByString<TextView>("phraseTextView")

        // ... more assertions and event triggering ...
    }
hatepoint commented 1 year ago

I used the recyclerView.assertSingleListItem() in Stage 2. It seems to work fine, but at this point all I needed from this method is to check ID's of Views in recyclerview.

RedJocker commented 1 year ago

yeah, that is ok, this issue is closed