Repository navigation
Fix time based conditions bug - #103
Merged
Merged
Conversation
Minor style amendments that might even bring some (unnoticeable) performance improvement
This commit deeply refactors the command based items in order to use the stable version of the `subprocess` crate, which has changed significantly and doesn't support `Popen` objects. Also, dependency versions in Cargo.toml have been loosened, although this does not change the actual way the dependencies are chosen.
Force time based condition to be verified just AFTER the related time specification has passed and is compliant with the specified instant, instead of being successful just BEFORE
almostearthling
added
bug
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
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.
This branch fixes a bug that has always been there, and that I wasn't able to discover until I started using time based conditions whose related tasks were short: the end time started being either too close to the presumed startup time, or even to happen before it: this breaks the "contract" stated in the documentation for which conditions are teested (and thus tasks are run) after a certain condition is verified.
Due to an incorrect calculation, the test was successful within the tick before the specified time instead of the one that took place after it: the calculation is now fixed.
Also, an unlikely (yet possible) chance to completely skip the verification of a time based condition, which required the machine to be very fast and the load close to zero, has been corrected.
Current setup is also less strict on crate dependencies, so that dependency updates are more likely to be accepted: this might cost something in terms of development, in order to be always compatible with all the dependencies, with the advantage of trying to mostly use the most up-to-date versions of the libraries: for instance, some of the code that involves process spawning (i.e. the one used for command based items) has been adapted to the stable version of the subprocess library, instead of requiring the deprecated one, and this led to more compact code and a slightly smaller binary.