Filter the tests results by outcome #issue38 #46
Conversation
Corrected the problem of the run line
|
@RibeiroAna you don't need to close and open new pull requests when you modify your code. If you push additional commits to the same branch, GitHub will show the changes in the open pull request. Once we're ready to merge we can squash the commits if needed. |
| html.td(additional_html, class_='extra')], | ||
| class_=result.lower() + ' results-table-row')) | ||
| html.td(additional_html, class_='extra', id=test_id)], | ||
| class_=result.lower() + ' results-table-row', id=test_id)) |
There was a problem hiding this comment.
We shouldn't have two elements with the same id.
The id global attribute defines a unique identifier (ID) which must be unique in the whole document.
From: https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/id
There was a problem hiding this comment.
You could consider using data attributes instead.
|
In testing this, I'm unable to filter out errors. I suspect this is related to differences in the test id. Here are some examples for generating setup/teardown errors: @pytest.fixture
def startup_error():
raise Exception('startup_error')
def test_startup_error(startup_error):
pass
@pytest.fixture
def teardown_error(request):
def fin():
raise Exception('teardown_error')
request.addfinalizer(fin)
def test_teardown_error(teardown_error):
pass |
| @@ -37,6 +48,7 @@ def assert_summary(html, tests=1, duration=None, passed=1, skipped=0, failed=0, | |||
| assert int(re.search('(\d)+ errors', html).group(1)) == errors | |||
| assert int(re.search('(\d)+ expected failures', html).group(1)) == xfailed | |||
| assert int(re.search('(\d)+ unexpected passes', html).group(1)) == xpassed | |||
| assert_summary_checkboxes(html, passed) | |||
There was a problem hiding this comment.
Rather than call another method and only check passed, it wouldn't be much additional code to check all boxes are in the HTML and have the appropriate state. You could potentially use **kwargs to loop over the outcomes passed to this method and reduce duplication. You might then need a dict for mapping xfailed to expected failures, etc.
There was a problem hiding this comment.
I think it may make sense to also add a test that opens the HTML report in a browser and verifies that toggling the checkboxes filters the results. This would have caught the issue I'm seeing with filtering errors.
| @@ -215,23 +218,51 @@ def pytest_sessionfinish(self, session): | |||
| html.title('Test Report'), | |||
| html.style(raw(style_css))) | |||
|
|
|||
|
|
|||
| class ResultByType: | |||
There was a problem hiding this comment.
I have no idea if here would be the best place for a new class
There was a problem hiding this comment.
This is fine. We could also move it to it's own file. In this case is the type the outcome? I'm a bit confused by the name of the class. This object represents each potential outcome, perhaps Outcome would be a better name?
| @@ -141,21 +141,26 @@ def _appendrow(self, result, report): | |||
| log.append(html.br()) | |||
| log.append(content) | |||
| else: | |||
| log = html.div(class_='empty log') | |||
| log = html.div(class_='empty log', colspan="5") | |||
There was a problem hiding this comment.
The colspan should be on the <td>, and not needed here.
|
The sort is only working for me every other click. Also, could you add a message to the page when all results have been filtered out? I've added a few other comments, but this is looking great! |
|
|
||
| class TestHTML: | ||
| # Asserts by outcome | ||
| assert_results_by_outcome(html, 'passed', passed) |
There was a problem hiding this comment.
@davehunt by doing that I don't have to write all the individual checks again
|
|
||
| # Asserts number of rows of the table | ||
| assert len(re.findall('tbody', html)) or 0 == tests |
There was a problem hiding this comment.
The or 0 should not be needed, as findall will return an empty list if there are no matches.
| }; | ||
| } | ||
|
|
||
| function key_num(col_index) { | ||
| return function(elem) { | ||
| return parseFloat(elem.childNodes[col_index].firstChild.data); | ||
| return parseFloat(elem.childNodes[1].childNodes[col_index].firstChild.data); | ||
| }; |
There was a problem hiding this comment.
@davehunt the problem was that before we had just one child (like, it was , but now it is first one about the rowlogs), so there was one more level to access.
There was a problem hiding this comment.
Aha! I knew you'd work it out. 😀
| label=None): | ||
| # Asserts if the test number of this outcome in the summary is correct | ||
| regex_summary = '(\d)+ {0}'.format(label or test_outcome) | ||
| int(re.search(regex_summary, html).group(1)) == test_outcome_number |
There was a problem hiding this comment.
Still missing an assert here.
|
This is looking great, @RibeiroAna! Could you add a message to the table when the user filters all the results out? Just something like "No results? Try removing some of the filters." In a later enhancement we could even add a "clear all" link to the message to remove any applied filters. |
|
Hi @davehunt , I just put the "not found" message. We could open a new issue to "clear all" function and merge that one =) |
I agree. I'll look at your latest changes first thing tomorrow. Thanks! |
| }); | ||
| function is_all_rows_hidden() { | ||
| var rows = find_all('.results-table-row'); | ||
| if(rows.length == 0) |
There was a problem hiding this comment.
Could you use a filter here, to reduce the array to only rows that are not hidden. Then you can just return the result of checking the length against zero.
|
@RibeiroAna just three comments to address, and then I think we're ready to merge. Rather than waiting on me, please feel free to move onto #49 (which I think is a great idea), or #37. |
How it looks: when the page is first loaded, there are checked check-boxes and by checking and unchecking the boxes, the users can filter which tests they want to see. If there are no tests of one outcome, the checkbox is disabled.