fix: IsFinished should return false when a task has not started - #2183
Open
MateusMo wants to merge 1 commit into
Open
fix: IsFinished should return false when a task has not started#2183MateusMo wants to merge 1 commit into
MateusMo wants to merge 1 commit into
Conversation
Author
|
@microsoft-github-policy-service agree |
Contributor
|
Thanks! Due to the very long "Changes" section I need to ask; there was no AI involved in this PR? |
Author
|
Yes, I found the behavior running a console app calling the method and used claude ai to help me to validate the scenarios and improve the test coverage |
Author
|
Any update on this one? Happy to make changes if needed |
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.
Fixes #2182
Changes
ProgressContext.IsFinishedpreviously used_tasks.Where(x => x.IsStarted).All(task => task.IsFinished). When no task in the context had started yet (for example, tasks created withautoStart: false),Where(...)produced an empty sequence, and.All()on an empty sequence always returnstrue(vacuous truth). This causedIsFinishedto reporttrueeven though no work had actually run, silently breaking the commonwhile (!ctx.IsFinished)usage pattern.ProgressContext.cs:IsFinishednow returnstrueonly when there are no tasks in the context, or when every task's ownIsFinishedistrue. SinceProgressTask.IsFinishedisfalsefor any task that hasn't reached itsMaxValue(including one that hasn't started), this naturally treats "not started" as "not finished" without needing a separateIsStartedcheck.ProgressTests.cs: added four regression tests covering: no tasks in the context, a single task withautoStart: false, a task that has started and finished, and a mix of one finished task alongside one not-started task.Please upvote 👍 this pull request if you are interested in it.