Increase pytest coverage by either adding tests or removing unused code #31
Conversation
marco-miller
changed the title
|
My latest force-push (diff linked above) here only rebased this PR on #28, still per description at top above. |
marco-miller
changed the title
Base this new test on the existing test_fetch_xy. Rename the hereby reused REQUESTED_TIME constants accordingly; no longer XY-specific. Don't redundantly assert model_type; test_fetch_timegraph_tree does so already. Now, adding this test increases test coverage (cf. README) in these specific ways: - time_graph_model.py: from 25% to 33%; - entry_model.py: from 84% to 88%; - response.py: from 86% to 89%. <=> From 59% to 60% total. More work remains then to cover tsp/ further. Signed-off-by: Marco Miller <[email protected]>
Remove the duplication introduced by test_fetch_timegraph_tree_complete, which was based on test_fetch_xy. Add the private __requested_parameters helper method for this. Signed-off-by: Marco Miller <[email protected]>
Signed-off-by: Marco Miller <[email protected]>
If some data is needed to be transferred between server and clients over the TSP, then it needs to properly be specified in the TSP, so that clients and server can support it. Fixes #35 Signed-off-by: Marco Miller <[email protected]>
Signed-off-by: Marco Miller <[email protected]>
Move tree_model.py's whole content (both classes) out of the tsp/ package, as only ./tsp-cli-client uses it. Make the file a sibling of the latter, as duly expected. This move also solves the issue of tsp/ pytest coverage previously reporting that file as not covered at all. Signed-off-by: Marco Miller <[email protected]>
The pytest(s) coverage and its reporting is not meant to cover all paths. Add no-cover pragma directives to uncovered paths that may be safely ignored by pytest reporting. Do so as the scope of these tests is mainly integration and main coverage of client paths towards the server. Ignore paths such as negative or corner (thus less likely) cases. Ignore paths also that are either TODO code or manual cli printouts and cases. The remaining uncovered lines are backlogged to be covered further, yet through larger efforts. README has the pytest command showing the report, which currently shows 80% coverage (round number a coincidence). Some no-cover pragmas might be removed as some more paths end up being covered, through potential pytest or code amends. Signed-off-by: Marco Miller <[email protected]>
|
@MatthewKhouzam and @bhufmann, this PR contains only its own diffs now, ready for your review; thanks. |
@marco-miller The code looks good to me. I'm ok to remove the parsing and storing of the unspecified parameters in field 'others' (according to #35). For the removal "DataType", I think for now it's ok to remove it, and it can be added (or something similar) back once there is a way forward for eclipse-cdt-cloud/trace-server-protocol#50. The PR message however, doesn't fully reflect all the changes done in this PR. Could you please give a brief summary in the PR description? |
Ack. Thanks for the review.
Done; thanks. |
timegraphtree test inTestTspClient.DataTypeclass with its whole file.EntryModel.Tree{Model,Item}out of thetsppackage, beingclionly.pragmas where there is no pytest coverage necessary yet.