Exit status refactoring & differentiate originator of failign exit code #906
Conversation
This file contains 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
This method was giant spaghetti, that was hard to maintain and fix stuff in. The new solution might seem overblown but with it's current interdependencies was the best I could come up with. Decided to decouple it a bit from asking SimpleCov itself for the coverage values to make it easier to test in isolation. No tests added as of right now though since it's tested quite a bit. The strategy to put each check into its own class produces more lines of code but makes their intent clearer and also makes it easier to figure out what belongs to what check.
Don't have to pull the value through basically everything. Also, for us that's quite the important concept to have centralized. I'm not a fan of where it lives right now, but might come up with something new and better.
Makes it easier to follow the code and there's no magic runnign there. Also makes our interface smaller.
Refactoring this part made it clearer that all the different checks against success weren't necessary and could be done on a higher level removing lots and lots of branches on the lower level all checking for the same thing. Also implements the feature that we print different error messages for aborting on "we want to throw an error" vs. "we determined there was an error before". Decided to print out both in case we have a false positive then people can file better error reports of "you said there was an error before, but there wasn't!" Also deleted some tests as they dealt with cases that don't happen anymore/don't have to be checked anymore. Added some cukes to show that we say something else went wrong. Test suit holds up. Let's hope this won't be another billion fixes for minute details I forgot about and need to fix after the next release!
PragTob
changed the title
When I accidentally fixed something, I hope I didn't accidentally break things.
Currently waiting to hear about: grosser/parallel_tests#772
|
Alright, merging. There are still some things left to do around parallel tests but I'll do that in a separate PR. |
|
Whoop! 🕺 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Basically a semi precursor to reviving #739 as I didn't want to meddle with this code.
Individual commits have more information.