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

improve tests structure (visibility and clearness of arrange/act/assert blocks) #313

Open
yashaka opened this issue Apr 11, 2021 · 7 comments

Comments

@yashaka
Copy link
Owner

yashaka commented Apr 11, 2021

This is a parent issue. For PR, break it into issues per test file to refactor, once created – link them. Send PRs - one per test file!

Imagine you have this kind of test:

    page = GivenPage(session_browser.driver)
    page.opened_empty()

    elements = session_browser.all('.will-appear')

    page.load_body(
        '''
                   <ul>Hello to:
                       <li class='will-appear'>Bob</li>
                       <li class='will-appear' style='display:none'>Kate</li>
                   </ul>'''
    )

    assert len(elements) == 2

It's unclear from first sight where is precondition, where this step and assert blocks (aka arrange/act/assert or given/when/then). This influces its readability, let's improve it to the following style with clear break down into 3 corresponding blocks:

def test_counts_invisible_tasks(session_browser):
    page = GivenPage(session_browser.driver)
    page.opened_empty()
    page.load_body(
        '''
        <ul>Hello to:
            <li class='will-appear'>Bob</li>
            <li class='will-appear' style='display:none'>Kate</li>
        </ul>
        '''
    )
    elements = session_browser.all('.will-appear')
    
    length = len(elements)

    assert length == 2
@aleksandr-kotlyar
Copy link
Collaborator

aleksandr-kotlyar commented Apr 11, 2021

Before refactor this - need to review a huge PR #283 with test migration from selene version 1 to selene version 2.

Update: #283 has been merged as is by agreement with @yashaka.

@rina-tenitska
Copy link
Collaborator

I will do

@yashaka
Copy link
Owner Author

yashaka commented Apr 19, 2021

@rina-tenitska, great!
Please, refactor files – 1 by 1 and each time:

Other contributors can do the same by the way, this is a big task, and can be done by more than 1 contributor ;)

@yashaka
Copy link
Owner Author

yashaka commented Apr 20, 2021

let's also reduce the scope of this issue just to tests inside https://github.com/yashaka/selene/tree/master/tests/integration

yashaka added a commit that referenced this issue Jun 19, 2021
fix #328, update #313 by Merge pull request #337 from igorvlasovkram/IgVlKr_1
yashaka added a commit that referenced this issue Jun 19, 2021
fix #356 by Merge pull request #357 from pupsikpic/master
@Dbhardwaj99
Copy link

Heyy, Is this issue still open? Would Like to contribute

@yashaka
Copy link
Owner Author

yashaka commented Jun 25, 2022

@Dbhardwaj99
I don't remember:) It might be still relevant :) Need to check)
But if you find something to improve in context of test structure of some tests, propose it here!

@Dbhardwaj99
Copy link

Sure

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants