BeautifulSoup logic in separate file #56
Conversation
| if no object is found for the given selector | ||
| """ | ||
| for arg in args: | ||
| selectedElems = html.select_one(arg) |
There was a problem hiding this comment.
won't it always take the last output of the select_one ? It's not adding, for every iteration it re-assigns selectedElems with the new value. No? or is it expected?
There was a problem hiding this comment.
That is what is expected. Each new iteration overrides a variable selectedElems
There was a problem hiding this comment.
then I don't get it, does it supposed to return:
- all objects for all found selectors?
- the last found object?
- the first found object?
- any object?
- is it expected to be some kind of chaining action, when the result of the previous iteration is used in the next one?
There was a problem hiding this comment.
then I don't get it, does it supposed to return:
we take the object, apply the method select_one with selector 1 to it, then apply the method select_one with selector 2 to the received object, then apply the method select_one with the selector X to the received object and return the object or False
There was a problem hiding this comment.
could you please provide real example from smarsy website? expected call with expected result?
There was a problem hiding this comment.
From issue#51
<TD valign=top align="left" width="120"><img src="https://smarsy.ua/images/mypage/parent_1.png"></TD>
We must find td with valign=top and in received object find img[src]
There was a problem hiding this comment.
and what would be the function call for that html?
There was a problem hiding this comment.
html.select_one([valign=top]).select_one('img[src]')
There was a problem hiding this comment.
will it work? Additionally here you are passing single value parameter, but in your function you are expecting array. Please provide an example with array
tests/test_bs_helper.py
Outdated
| self.assertEqual(actual, 'some text') | ||
|
|
||
| def test_bs_safe_select_return_expected_text_with_many_selectors(self): | ||
| selector1, selector2, selector3 = 'some_tag1', 'some_tag2', 'some_tag3' |
There was a problem hiding this comment.
I don't understand this test. It works with any selector values and with any quantity of them. What's the purpose of it?
There was a problem hiding this comment.
not sure what was done for this one
There was a problem hiding this comment.
I refactored this test
tests/test_bs_helper.py
Outdated
| self.mocked_soup.get.return_value = 'some text' | ||
| actual = self.source_page.bs_safe_get(self.mocked_soup, | ||
| 'some attribute') | ||
| self.assertEqual(actual, 'some text') |
There was a problem hiding this comment.
as you are testing the fact that you are getting the output of the get function, then you should use self.mocked_soup.get.return_value instead of hardcoding some text in the assert
There was a problem hiding this comment.
Do you saw latest commit?
def test_bs_safe_get_return_expected_text(self):
actual = self.source_page.bs_safe_get(self.mocked_soup, self.expected_attribute)
self.assertEqual(actual, self.expected_text)
|
|
||
| def test_bs_safe_select_return_expected_text_with_many_selectors(self): | ||
| select_one = None | ||
| for select_one_value in self.select_one_values: |
There was a problem hiding this comment.
the test is re-producing the same logic as in the function that is not good. This test is bad, but before answering what exactly is bad you need to answer the question posted under this function's source code.
…bs_safe_get_return_false_
dkultasev
force-pushed
the
issue-55-Move_out_Be_logic_to_sep_PR
branch
from
fe9959d to
06a5df1
Compare
No description provided.