Add op/extract/split Unit Test #186
Conversation
Sdddell
requested review from
CambioML,
goldmermaid,
SayaZhang,
CluckRookie and
SeisSerenata
as code owners
|
@Sdddell It looks like the build is failing due to formatting issue. Please make sure you follow https://www.notion.so/goldpiggy/Python-Linter-and-formatter-Setup-30fb3b81f0904af889832e4c697c5ec9?pvs=4 to properly setup your vs code, reformat, and submit your PR again. |
| def setUp(self): | ||
| self.splitter = MarkdownHeaderSplitter("test_splitter") | ||
|
|
||
| def test_call(self): |
There was a problem hiding this comment.
provide a meaningful function name
There was a problem hiding this comment.
It is testing the special method def __call__(self, x). Should I just rename it to test___call__() or test_speical_call()?
There was a problem hiding this comment.
maybe call it test_callable_object_construction
| self.assertEqual(output_nodes[0].value_dict["text"], ["# Header ## Content"]) | ||
| self.assertEqual(output_nodes[1].value_dict["text"], ["# Header", "## Content"]) | ||
|
|
||
| def test_header_splitter(self): |
There was a problem hiding this comment.
can split into two test cases if you are intended to test single_header and multiple_header:
test_header_splitter_with_single_header
test_header_splitter_with_multiple_header
There was a problem hiding this comment.
It's testing processing Sequence[Node].
| markdown_str = " <h1># Header</h1> \n Content" | ||
| headers_to_split_on_list0 = [] | ||
| headers_to_split_on_list1 = [("\n", "h2")] | ||
| headers_to_split_on_list2 = [("<h1>", "h1")] |
There was a problem hiding this comment.
what does headers_to_split_on_listx mean here?
| self.assertEqual(result, ['<h1># Header</h1>\nContent']) | ||
| self.assertEqual(result0, []) | ||
| self.assertEqual(result1, []) | ||
| # self.assertEqual(result2, ['<h1># Header</h1>\nContent']) |
| def setUp(self): | ||
| self.splitter = PatternSplitter("test_splitter") | ||
|
|
||
| def test_call(self): |
There was a problem hiding this comment.
ditto: provide a meaningful function name
There was a problem hiding this comment.
maybe call it test_callable_object_construction
|
Fix styles. |
|
The failing test case is about checking if |
@SayaZhang Could you please help to take a look. |
|
Remove custom html-style header test of markdown header splitter. |
| def test_list(self): | ||
| splitters = SplitterOpsFactory.list() | ||
|
|
||
| self.assertEqual(len(splitters), 3) |
There was a problem hiding this comment.
maybe try assertListEqual or assertEqual directly rather than comparing element by element
|
|
||
| output_nodes = self.splitter([node0, node1, node2, node3]) | ||
|
|
||
| self.assertEqual(len(output_nodes), 4) |
There was a problem hiding this comment.
same here maybe try assertEqual directly rather than comparing element by element
|
LGTM |
Add the following tests:
<h1> xx </h1>