Skip to content

Conversation

@cce
Copy link
Contributor

@cce cce commented Oct 23, 2025

Summary

In the multisig e2e tests, msgpacktool -d | grep -q runs under set -o pipefail, so when grep -q exits right after matching "lmsig"/"msig", msgpacktool might still be writing and encounter EPIPE, leading to a non-zero status reported and a failed test.

Test Plan

Tests should be hopefully less flaky.

@cce cce force-pushed the flaky-e2e-multisig branch from ade26d4 to d615185 Compare October 23, 2025 18:40
@cce cce added the Bug-Fix label Oct 23, 2025
@cce cce requested review from algorandskiy and Copilot October 24, 2025 00:43
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 fixes flaky e2e multisig tests caused by race conditions when piping msgpacktool -d output to grep -q. The issue occurs because grep -q exits immediately after finding a match, which can cause msgpacktool to receive EPIPE while still writing, resulting in non-zero exit status under set -o pipefail.

Key Changes:

  • Split the pipeline operation into two separate steps: decode to file, then grep the file
  • Eliminates the race condition by ensuring msgpacktool completes before grep runs

Reviewed Changes

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

File Description
test/scripts/e2e_subs/v32/e2e-teal-multisig.sh Breaks pipeline into separate decode and grep steps for v32 legacy mode test
test/scripts/e2e_subs/e2e-teal-multisig.sh Breaks pipeline into separate decode and grep steps for future consensus mode test

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

@cce cce requested review from gmalouf and jannotti October 24, 2025 16:49
Copy link
Contributor

@jannotti jannotti left a comment

Choose a reason for hiding this comment

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

We should probably comment on this, so that someone doesn't tidy it up some day to avoid the temp file.

@gmalouf gmalouf merged commit 9dc5156 into algorand:master Oct 24, 2025
62 of 65 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants