Skip to content

docs: audit delay and sleep sites before further conversions - #155

Merged
dborgards merged 5 commits into
mainfrom
db/114-delay-sleep-audit-2cb1
Sep 25, 2026
Merged

dborgards merged 5 commits into
mainfrom
db/114-delay-sleep-audit-2cb1

Conversation

@dborgards

@dborgards dborgards commented Sep 25, 2026 •

Copy link
Copy Markdown
Owner

What does this change?

Counts and classifies every Task.Delay / Thread.Sleep under tests/CanKit.Pro.Tests/TestCases, plus the WithTimeout budgets a sleep grep misses, so the next virtual-clock conversion has a list instead of discovering the next flake by turning CI red. No test is converted and no tolerance is widened.

Closes #114.

Type of change

  • feat — new behaviour (minor release)
  • fix / perf — bug or performance fix (patch release)
  • docs / test / refactor / chore / ci — no release
  • Breaking change (! in the title, plus a BREAKING CHANGE: footer explaining the migration)

Checklist

  • dotnet build CanKit.Pro.sln -c Release succeeds
  • dotnet test CanKit.Pro.sln -c Release passes
  • Public API changes are documented with XML comments
  • New behaviour is covered by a test
  • The requirement or ADR this relates to is referenced (e.g. FR-RAW-031, ADR-7), if any

CI on d9fb37c is green (ubuntu, windows, macOS). The previous commit of this branch failed macos-latest once on UdsClientTests.A_Pending_Answer_Consumed_As_Another_Requests_Stray_Still_Extends_Its_Window (P2 timeout after 600 ms for service 0x11, run 36102259889). Ubuntu and Windows passed that run. The test is already in the category-2 table; its handler sleeps 400 ms inside a 600 ms P2. It is not fixed here.

This branch adds docs/reviews/2026-09-25-delay-sleep-audit.md only. Reviews are excluded from the MkDocs site.

On c359bf0 there are 195 real delay/sleep call sites (201 grep hits, 6 of them comments; the issue's 117 is the older grep). 115 sites in 99 tests assume work finished inside a window (category 2). One sleep is itself a whole-test budget, and one non-sleep budget is still tight: StartPeriodicSend_SingleFrame_FiresAtConfiguredPeriod collects 22 samples at 120 ms inside 10.56 s (~1.4× if each period stretches to 3×). J1939TpChannel still builds its own actor with no clock; CanOpenNode now takes an ITimeSource but still builds the actor. Suggested first conversions are the J1939 backoff sleeps that already have a virtual-clock sibling, then that periodic budget. Bracket from both sides, and do not advance a clock until the timer is armed.

Open in Web Open in Cursor 

Count and classify Task.Delay, Thread.Sleep, and whole-test
timeouts under TestCases. No test is converted.

Closes #114

Co-authored-by: Dietmar Borgards <dborgards@users.noreply.github.com>
@codecov

codecov Bot commented Sep 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

The failure is a category-2 UDS sleep racing P2. This branch
does not change that test.

Co-authored-by: Dietmar Borgards <dborgards@users.noreply.github.com>
@dborgards
dborgards marked this pull request as ready for review September 25, 2026 16:51
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-25T17:10:12.112962Z e3a8e6e New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d9fb37cc7f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread docs/reviews/2026-09-25-delay-sleep-audit.md Outdated
Comment thread docs/reviews/2026-09-25-delay-sleep-audit.md Outdated
cursoragent and others added 2 commits September 25, 2026 17:05
The ISO-TP handoff sleep waits on CanBusService's send lock, and the
nine NMT-Start sleeps wait on the actor mailbox. Neither is an
ITimeSource conversion.

Closes #114

Co-authored-by: Dietmar Borgards <dborgards@users.noreply.github.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e3a8e6e022

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread docs/reviews/2026-09-25-delay-sleep-audit.md Outdated
Comment thread docs/reviews/2026-09-25-delay-sleep-audit.md Outdated
Comment thread docs/reviews/2026-09-25-delay-sleep-audit.md Outdated
CANopen session, wire, and pump sleeps are not an ITimeSource
conversion. UDS lock, queue, and pump sleeps are not the client
time seam. A green site does not need a failing load run first.

Closes #114

Co-authored-by: Dietmar Borgards <dborgards@users.noreply.github.com>
@dborgards
dborgards merged commit 9b21553 into main Sep 25, 2026
21 of 22 checks passed
@dborgards
dborgards deleted the db/114-delay-sleep-audit-2cb1 branch September 25, 2026 19:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Audit the test suite's 117 delay/sleep sites before converting any more of them

2 participants