Skip to content

Improved tests structure in files#355

Open
Elena7788 wants to merge 2 commits intoyashaka:masterfrom
Elena7788:#352-353_improving_test_structure
Open

Improved tests structure in files#355
Elena7788 wants to merge 2 commits intoyashaka:masterfrom
Elena7788:#352-353_improving_test_structure

Conversation

@Elena7788
Copy link
Copy Markdown

Improved tests' structure in element__element__lazy_search_test.py and element__lazy_search_test.py

Copy link
Copy Markdown
Owner

@yashaka yashaka left a comment

Choose a reason for hiding this comment

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

please refactor according to comments;)

</h1>
'''
)
answer = element().is_displayed()
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

this is an act, it need an empty line before... (the load_body is not an act, it's an arrange clause)

).element('.will-exist-inner')
page = GivenPage(session_browser.driver)
page.opened_empty()

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

this emply line is wrong, the next page load is also an arrange

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

page.load_body(
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

you don't need in this case separation between opened_empty() and load_page(), you can use the .opened_with_body() instead, that covers both previous ones

the separation was needed when element = ... was after opened_empty(), but not it is in the beginning of the test...

@@ -57,17 +58,19 @@ def test_search_is_postponed_until_actual_action_like_questioning_displayed(
def test_search_is_updated_on_next_actual_action_like_questioning_displayed(
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

please watch this: https://youtu.be/47yhzbiCJSw?t=1147
and refactor the test below, it should have same structure as in video;
assert should be broken into ACT and ASSERT clauses, and all previous page loading including first store of "answer" should be in ARRANGE

also
I would use the update word instead of new for consistency with test name; all this was in video;) watch it!

<h1 id="first">Heading 1</h1>
<h2 id="second">Heading 2</h2>
/p>'''
/p>
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

should be

</p>

'''
)

assert element().is_displayed() is True
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

should be brokent into ACT + ASSERT with additional variable with answer

@@ -50,11 +59,22 @@ def test_search_is_updated_on_next_actual_action_like_questioning_displayed(
page = GivenPage(session_browser.driver)
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

same logic should be applied as in video mentioned earlier

'<h1 id="will-be-existing-element-id" style="display:none">Hello kitty:*</h1>'
'''
<h1 id="will-be-existing-element-id" style="display:none">Hello kitty:
*</h1>
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

should be broken with a common sense, not just at some point...

good version is more like this:

<h1 id="will-be-existing-element-id" style="display:none">
    Hello kitty:*
</h1>

'''
)

assert element().is_displayed() is False
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

same as in first file...

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants