Skip to content

Conversation

@cce
Copy link
Contributor

@cce cce commented Oct 7, 2025

Summary

The change in #6455 removed msgp.Raw from the message type which meant errSkipRawMsgpTesting was no longer returned when calling protocol.RandomizeObject in TestRandomizedEncodingFullDiskState. This made the test 7x slower (~75s to ~530s). These changes hope to bound the number of iterations and size of random maps/slices made to keep the test times faster. In this PR it takes 27s.

Test Plan

Existing test should pass and not take longer than 10m

gmalouf
gmalouf previously approved these changes Oct 8, 2025
@cce cce requested a review from Copilot October 8, 2025 14:41
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR optimizes the TestRandomizedEncodingFullDiskState test which became 7x slower after a previous change. The optimization adds bounds to randomized collection sizes and provides configuration options to silence warning messages during testing.

  • Added configuration options to limit randomized slice/map lengths and silence allocation warnings
  • Modified test to use smaller collection sizes (8 instead of 32) and fewer iterations (1000/500 instead of 5000)
  • Updated randomization functions to accept and use the new configuration parameters

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
protocol/codec_tester.go Added configuration options for max collection length and silencing warnings, updated randomization logic to use these bounds
agreement/persistence_test.go Applied new configuration options to limit test execution time and reduce collection sizes

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@cce cce requested a review from gmalouf October 8, 2025 17:04
@cce cce merged commit 31eac08 into algorand:master Oct 8, 2025
39 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants