AAP-45541 Add test to recreate jobs/4075584/job_events/children_summary/ error #16163
Conversation
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
Member
from test failure, not the same as your expectation? |
Member
Author
|
Accidentally left a breakpoint() in the code. Think of this PR as pseudo code. My point was to show creating an event with event='' recreates the reported error. |
Member
|
Now it gives this which is intended: |
* Also begin to add detection for empty event
| ).save() | ||
| JobEvent.create_from_data(job_id=job.pk, uuid='uuid4', parent_uuid='', event='verbose', counter=4, stdout='a' * 1024, job_created=job.created).save() | ||
|
|
||
| JobEvent.create_from_data(job_id=job.pk, uuid='uuid4', parent_uuid='', event='', counter=5, stdout='a' * 1024, job_created=job.created).save() |
There was a problem hiding this comment.
Bug: Duplicate uuid in test creates incorrect data mapping
The empty event at line 163 uses uuid='uuid4', which duplicates the uuid from the verbose event at line 161. In the existing test patterns, each event has a unique uuid. This causes the map_uuid_counter dictionary to map uuid4 to counter 5 (empty event) instead of counter 4 (verbose event), since the later event overwrites the earlier one. While this doesn't break the current test (no event uses parent_uuid='uuid4'), it creates inconsistent test data that could mask bugs or cause failures if the test is extended.
|
AlanCoding
reviewed
Dec 17, 2025
| for i, e in enumerate(events): | ||
| if not e['event']: | ||
| logging.warning(f'event type missing for event {e}') | ||
| continue |
Member
There was a problem hiding this comment.
I think my only remaining question is if this continue is necessary or if we could just handle these events within the rest of the logic. But either way.
AlanCoding
approved these changes
Dec 17, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
I'm not sure how we can get into this state (where event is empty). Best we can do for now (until we get an actual reproducer) is log when it happens and try not to include/process the malformed event so that we still return a usuable event tree.
SUMMARY
ISSUE TYPE
COMPONENT NAME
ADDITIONAL INFORMATION
Note
Handle empty/missing job event types in job_events children_summary with safe skipping and logging; add functional test coverage.
eventinJobJobEventsChildrenSummary.get, logging a warning instead of erroring.''as meta when scanning ahead to determine parentage for meta events; avoidKeyErrorand preserve tree processing.test_job_job_events_children_summary_empty_eventto verify correct behavior when an event has an emptyeventfield.Written by Cursor Bugbot for commit 3f251e0. This will update automatically on new commits. Configure here.