gh_322: Improve test structure #323
Conversation
| @@ -38,34 +38,37 @@ def test_search_is_postponed_until_actual_action_like_questioning_count( | |||
| ): | |||
| page = GivenPage(session_browser.driver) | |||
| page.opened_empty() | |||
|
|
|||
| elements = session_browser.element('ul').all('.will-appear') | |||
There was a problem hiding this comment.
moving this line further is wrong, because the main idea of this test, is to show, that search of actual element will not appear before loading actual body with .will-appear elements...
| page.load_body( | ||
| ''' | ||
| <ul>Hello to: | ||
| <li class='will-appear'>Bob</li> | ||
| <li class='will-appear'>Kate</li> | ||
| </ul>''' | ||
| ) | ||
| elements = session_browser.element('ul').all('.will-appear') | ||
|
|
||
| length = len(elements) |
There was a problem hiding this comment.
the test name already uses word "count" for this context...
that's why it would be more consistent and readable to name this variable as count too;)
|
|
||
|
|
||
| def test_search_is_updated_on_next_actual_action_like_questioning_count( | ||
| session_browser, | ||
| ): | ||
| page = GivenPage(session_browser.driver) | ||
| page.opened_empty() | ||
| elements = session_browser.element('ul').all('.will-appear') |
There was a problem hiding this comment.
here, it's not like that important to keep this line here, because we nevertheless will ask for count twice, and only second time is a part of our test goal...
yet... also no much reason to move further...
would be simpler to keep this line here for kind of consistency with previous test structure...
|
|
||
| length = len(elements) | ||
|
|
||
| assert length == 2 |
There was a problem hiding this comment.
here we don't need an assert actually (because yet this is a precondition...)... we just need to store the current length (that should be better named as count)
we can, by the way, have asserts in preconditions for some reason... if catching some extra errors would be more optimal as early as possible... but at least - not obligatory... and maybe if moving this asserts to the end of test - would make test logic cleaner... and this assert in precondition would not distract us...
| @@ -76,14 +79,14 @@ def test_search_is_updated_on_next_actual_action_like_questioning_count( | |||
| </ul>''' | |||
| ) | |||
|
|
|||
| assert len(elements) == 3 | |||
| new_length = len(elements) | |||
There was a problem hiding this comment.
again, better to name as updated_count not new_... because the word "updated" is already used in test name
| assert len(elements) == 3 | ||
| new_length = len(elements) | ||
|
|
||
| assert new_length == 3 |
There was a problem hiding this comment.
can be tuned to something like assert 3 == updated_count != first_count
or
assert updated_count == 3
assert updated_count != first_count
| @@ -92,5 +95,8 @@ def test_searches_exactly_inside_parent(session_browser): | |||
| </ul> | |||
| <li class='forgotten'>Joe</li>''' | |||
| ) | |||
| elements = session_browser.element('ul').all('.will-appear') | |||
|
|
|||
| length = len(elements) | |||
There was a problem hiding this comment.
better to use count word for consistency with previous tests
|
|
||
|
|
||
| def test_searches_exactly_inside_parent(session_browser): | ||
| page = GivenPage(session_browser.driver) | ||
| page.opened_empty() | ||
|
|
||
| elements = session_browser.element('ul').all('.will-appear') |
There was a problem hiding this comment.
same as above... this line should be here (not moved further)
| @@ -38,34 +38,37 @@ def test_search_is_postponed_until_actual_action_like_questioning_count( | |||
| ): | |||
| page = GivenPage(session_browser.driver) | |||
| page.opened_empty() | |||
|
|
|||
| elements = session_browser.element('ul').all('.will-appear') | |||
| page.load_body( | |||
| ''' | |||
| <ul>Hello to: | |||
| <li class='will-appear'>Bob</li> | |||
| <li class='will-appear'>Kate</li> | |||
| </ul>''' | |||
There was a problem hiding this comment.
let's move last ''' to next line here, and below when needed... ;)
rodinant
force-pushed
the
issue_322_improve_test_structure
branch
from
332664d
to
eeb3e72
Compare
Codecov Report
@@ Coverage Diff @@
## master #323 +/- ##
==========================================
- Coverage 61.18% 61.07% -0.11%
==========================================
Files 35 35
Lines 1829 1829
Branches 117 117
==========================================
- Hits 1119 1117 -2
- Misses 682 684 +2
Partials 28 28
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
No description provided.