You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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.
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 #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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
As mentioned in issue #11, the
forget-resultscommand's default behaviour of deleting everything will be super dangerous if it ever gains support for deleting individual results. I have now added a--alloption 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-testsoption 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
--allnow 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.