fix: Allow ibazel notifications to pass when not running tests#52
Conversation
If `ibazel` notifies of a change or the context is cancelled, then deal with it when we are waiting for service errors or test completions. This avoids a deadlock in interactive modes. Fixes: hermeticbuild#51
This is a workaround that avoids the deadlock from the previous implementation, as that would not exit the drain loop until the channel was closed. Here we just read if we can in a best effort fashion. A better solution would be to let the writing goroutine close the channel on exit and for us to notify on close with a context.
| testStartTime := time.Now() | ||
|
|
||
| testCmd = exec.CommandContext(ctx, testPath, testArgs...) | ||
| testCmd = exec.CommandContext(testCtx, testPath, testArgs...) |
There was a problem hiding this comment.
@codex will this testCmd also get stopped/cancelled when the parent ctx is cancelled?
There was a problem hiding this comment.
When one context is derived from an outer context, as queryCtx is derived from ctx in this example, if the outer context is canceled, then the derived context is automatically canceled as well.
There was a problem hiding this comment.
Ah perfect, I suspected as much but was curious if I could get Codex to explain it. Guess not :)
|
@codex will the testCmd also get stopped/cancelled when the parent ctx is cancelled? please review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ 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".
|
@codex review |
|
Codex Review: Didn't find any major issues. Chef's kiss. ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
jaqx0r
force-pushed
the
notify-blocked
branch
from
06edcc9 to
b1237ec
Compare
|
Thanks for fixing this! |
|
Thanks for the speedy review! |
2 participants
This change merges two selects so that context cancellation and ibazel notifications are also checked when waiting for tests to run or for services to fail.
This helps in non-test situations like when using ibazel to shorten the edit/test cycle.
Fixes: #51