diff: Rework --quiet logic and handle file without needing --quiet #959
Conversation
|
@xukai92 PTAL when you get a chance, TIA |
The logic is organized so there is not separate work paths for quiet and non-quiet. The code now properly identifies and handles single files. Fixes instructlab#796 Signed-off-by: BJ Hargrave <hargrave@us.ibm.com>
I added 2 unit tests. One for diff on a file (vs. a repo) without --quiet and one on a file with --quiet. There are no functional tests on diff and the unit test cover diff quite well now. |
Am I reading your tests right that they create a new taxonomy file and run The tests lgtm, and I appreciate you adding them! I'm not even suggesting you need to do something different in this PR. It's more of a general comment. We need to organize tests to more cleanly separate what are functional tests and what are actual unit tests. |
Perhaps. Some things can be well tested in so-called unit tests. When things need to be choreographed in more detail, then so-called functional tests can be authored. The current functional tests are serialized. Most functional test methods do not depend upon previous test methods for some state. It is also awkward to run and debug the functional tests as there is no test harness. Given the updated unit tests now support calling the lab.cli function, I think we should consider redoing the function tests as pytest test methods replacing the bash script. |
That is what they do. Each new test method is a copy of the previous test method but changed to pass a file path instead of a repo path. I don't have an opinion on whether you call then unit test or functional tests, but I do think they are properly placed in the tests as pytest test methods. |
|
@Mergifyio refresh |
✅ Pull request refreshed |
|
@russellb This PR has 8 requested reviews and I am sure not all of them will happen. Since mergify requires no requested review to auto merge, can we dismiss the requested reviews or merge manually? |
Changes
Which issue is resolved by this Pull Request:
Resolves #796
Description of your changes:
The logic is organized so there is not separate work paths for quiet and non-quiet. The code now properly identifies and handles single files.
An exception in the non-quiet code when reading the repo now results in a failing exit code.