Add unit tests and partially rewrite method to pass tests. #45
Conversation
Jturnerusa
force-pushed
the
feature/pass-tests
branch
from
eb51d29 to
f39910e
Compare
|
I think I should make the test run in CI or something before merging this, let me know what you think, or if its too big of a change! 🚀 Edit: I put the test file in pym/gentoolkit/tests and the existing CI should pick up on it if it runs tests currently. |
Jturnerusa
force-pushed
the
feature/pass-tests
branch
from
f39910e to
749aa8a
Compare
|
Please fix CI (at least the pylint stuff) then rebase? |
|
Also, I'm a bit suspicious API-wise of 749aa8a. It's unusual to have a library print for you if it just has "format" in the name. Maybe rename it, or make a print one which calls it & port callers to that? |
I am a little confused about this, do you mean my change or do you mean the already existing API? I think maybe I didn't explain the change well enough in the commit message, but with my change, the depends.py equery module handles the "equery depends" specific formatting and printing logic, everything contained inside of it's own module, rather than expecting other modules to know anything about this. The depends.py module used to pass in this printer class to be used as a callback but otherwise the printer class was only used by depends.py and is very specific to the "equery depends" command. I don't think any of the classes or functions in depends.py should even be considered "public" since they are all specific and only used internally with the exception of the printer callback "hack" which is partially removed (there is another callback in the Dependencies.graph_depends method which I want to remove). To be clear, the Dependencies.py class is a more general gentoolkit class that is not "equery depends" specific, it's used in at least one other equery command (depgraph) and maybe in more places. I think this module shouldn't be worried about printing or formatting for equery commands, but it was before possibly for performance reasons(?). I might be confused about what you are talking about also and if so I am sorry! |
Jturnerusa
force-pushed
the
feature/pass-tests
branch
from
749aa8a to
0fd190d
Compare
Jturnerusa
force-pushed
the
feature/pass-tests
branch
6 times, most recently
from
57936b6 to
a2aef5e
Compare
|
Can you rebase, then it should be good to go? |
This commit introduces a new file with a basic unit test for the graph_reverse_depends method on the Dependencies class. Only 1 test exists right now, but various setup code and the general pattern of the tests are valuable for creating more advanced tests, and more tests for the Dependencies class in general. Pytest is used to run all of the functions in the file that start with test_, and a monkeypatch object is passed into test cases and allows us to mock out methods on the Dependencies class, specifically the "environment" method, which is used to query for packages variables like DEPEND and RDEPEND. We are able to test against a small fake denendency graph by patching the method! Signed-off-by: John Turner <[email protected]>
The graph_reverse_depends method was not able to pass the unit tests introduced in the previous commits. It has been rewritten to pass them. This also has adding types to the method, and yields the results as an iterator rather than collecting them into a list in one shot. The printer callback parameter has been removed. This callback most likely existed so that results would be shown to the user as soon as they were available instead of delaying printing until the method completed, which could take seconds or minutes depending on the parameters. By making this method an iterator, the same effect is acheived by having the caller print every item as its yielded from the method. Signed-off-by: John Turner <[email protected]>
The depends module can now iterate over the results of the graph_reverse_depends function and print the items as they are yielded. Before, it passed in a callback printer function, and expected the Dependencies class to call it correctly. This setup is nicer because it does not tie together this module and the Dependencies class, and the old setup most likely existed due to performance and interactivity concerns which are now fixed by turning graph_reverse_depends into an iterator. Signed-off-by: John Turner <[email protected]>
The DependPrinter class name and documentation indicated that it was meant to be part of the gentoolkit.dependencies API, to print Dependencies objects. This Printer class is used exclusively for printering equery depends output and is not a public API outside of the depends.py module Signed-off-by: John Turner <[email protected]>
Pytest is a testing framework that is backwards compatible with "unittest" tests, but provides new styles of tests that are more ergonomic. Pytest tests do not require wrapping the test in a class, just a top level python function will be automatically picked up. Assertions use the regular python assert built-in and provide greatly enhanced debug output. These features reduce friction in writing new unit tests, and being backwards compatible allows preserving the existing gentoolkit unit tests. Changing the meson test command and installing the pytest package in CI are the only changes required to start using it! Signed-off-by: John Turner <[email protected]>
Jturnerusa
force-pushed
the
feature/pass-tests
branch
from
a2aef5e to
63f3848
Compare
This PR will introduce pytest testing for the gentoolkit.dependencies.Dependencies class's graph_reverse_depends method. The method was not able to pass a trivial test and has been partially rewritten.
graph_reverse_depends has type annotations and is turned into an iterator, it also does not use the callback printer function which is used only for "equery depends".
The equery depends module is changed to handle printing on its own by iterating over the methods items and calling the printer function on the results.