git-test icon indicating copy to clipboard operation
git-test copied to clipboard

Add (weakly required) --all argument to forget-results as well as adding --all-tests argument

Open hlovdal opened this issue 3 years ago • 2 comments

As mentioned in issue #11, the forget-results command's default behaviour of deleting everything will be super dangerous if it ever gains support for deleting individual results. I have now added a --all option to that command which gives a warning if it is not given but otherwise the command behaves as before (except it now prints status a line on success).

In addition a bonus --all-tests option is also added that will as the name implies delete results for all the defined tests. So regardless of if the command ever gets support for deleting individual results, it now makes sense to be required to explicitly specify which tests that should be forgotten.

I have no plans on working on adding support for deleting individual test results with the forget-results command, but by adding --all now this should be less of a problem to do some time in the future.


When refactoring and splitting functions I let the code stay in the same place as before as much as possible to make the diffs small, but that might have resulted in a bit untidiness with regards to code layout in the file. Maybe you want to move around on some of the functions.

I have kept the colouring of the warning consistent with the existing code, but I really think it deserves a more prominent colour.

hlovdal avatar Mar 14 '21 22:03 hlovdal

I admit that cmd_list() was a kludge, but instead of turning its reading part into a separate function, I think it would be better to build a more robust way of reading tests. In particular, I don't want to lock us into thinking of a Test as just a command, because I think there will be value from adding other types of test as well (for example, I'd love to add a new type of test that causes multiple other tests to be run).

I've implemented what I think the reader abstraction could look like in https://github.com/mhagger/git-test/pull/19. Also, I implemented this as an iterator, because I think that makes it easier to use. What do you think of it? If it's OK with you, I'd rather have the functionality of this PR rewritten based on iter_tests() rather than your process_all_tests_one_by_one() function.

mhagger avatar Mar 24 '21 10:03 mhagger

Branch updated to make use of iter_test, as well as adding/updating tests for each relevant commit.

hlovdal avatar Aug 01 '21 22:08 hlovdal