Skip to content

Conversation

@varsill
Copy link
Contributor

@varsill varsill commented Apr 4, 2025

This PR:

  • Makes sure that all handle_child_terminated callbacks for children in crash group are called before handle_crash_group_down of that group
  • Adds test checking that assertions
  • Fix a formatting issue in documentation for the testing pipeline

TODO:

  • Make sure that terminated children is not in the context of handle_child_terminated

…e handle_crash_group_down. Add test checking that. Fix a formatting issue in documentation for the testing pipeline
@varsill varsill requested a review from mat-hek as a code owner April 4, 2025 13:38
@varsill varsill requested a review from FelonEkonom April 4, 2025 13:40
@varsill varsill marked this pull request as draft April 4, 2025 13:41
@varsill varsill marked this pull request as ready for review April 7, 2025 12:48
@varsill varsill added this to Smackore Apr 7, 2025
@varsill varsill moved this to In Review in Smackore Apr 7, 2025
@varsill varsill self-assigned this Apr 7, 2025
Comment on lines 69 to 79
def maybe_detonate_crash_group(_child_name, %CrashGroup{} = _group, :normal, state), do: state

def maybe_detonate_crash_group(
_child_name,
%CrashGroup{detonating?: true} = _group,
_crash_reason,
state
),
do: state

def maybe_detonate_crash_group(crash_initiator, %CrashGroup{} = group, crash_reason, state) do
Copy link
Member

Choose a reason for hiding this comment

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

you can match on detonating?: false and add a guard reason != :normal to have 2 cases instead of 3

@varsill varsill merged commit f8b7652 into master Apr 8, 2025
5 of 6 checks passed
@varsill varsill deleted the varsill/fix_crash_group_termination_callbacks_order branch April 8, 2025 14:52
@github-project-automation github-project-automation bot moved this from In Review to Done in Smackore Apr 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants