Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix process-integration-results.sh #6063

Merged
merged 4 commits into from
Aug 27, 2022

Conversation

mhofman
Copy link
Member

@mhofman mhofman commented Aug 26, 2022

refs: #6022

Description

Updates the processing script to handle some more cases:

  • Run without error if no snapshot were found to diverge
  • Remove new time/stats fields from slog for diffing
  • Generate an artifact when xsnap traces are found to diverge (to diagnose issues like in XS behavior divergence during CI #6022)

Security Considerations

None, manual debug scripts

Documentation Considerations

There should be some

Testing Considerations

Generated artifacts for these divergences in #6011 tests and on a local run of the same branch.

@mhofman mhofman requested a review from warner August 26, 2022 15:37
@mhofman
Copy link
Member Author

mhofman commented Aug 26, 2022

Best reviewed commit-by-commit as there is a refactor (move lines around) which clobbers the diff.

Copy link
Member

@warner warner left a comment

Choose a reason for hiding this comment

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

looks good, thanks

@@ -11,6 +11,10 @@ RESULTSDIR=${RESULTSDIR-"$NETWORK_NAME/results"}

[ $# -gt 0 ] && RESULTSDIR="$1"

clean_slog() {
jq -cr 'del(.time, .monotime, .dr[2].timestamps, .memoryUsage, .heapStats, .statsTime)'
Copy link
Member

Choose a reason for hiding this comment

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

be on the lookout for some kernelStats that @FUDCo will be adding to one of the cosmic-swingset end-block slog records, maybe next week, I think some of them are non-consensus

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah but I believe all of them are actually identical. Regardless they are already written to the swingstore and there is no diff showing up so far ;)

@mhofman mhofman added the automerge:rebase Automatically rebase updates, then merge label Aug 27, 2022
@mhofman mhofman force-pushed the mhofman/fix-process-integration-results branch from ab6d9fe to 7b5b2e8 Compare August 27, 2022 07:30
@mergify mergify bot merged commit 6494a03 into master Aug 27, 2022
@mergify mergify bot deleted the mhofman/fix-process-integration-results branch August 27, 2022 08:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge:rebase Automatically rebase updates, then merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants