Skip to content

Process: add wait_line and fix delta_only buffer offset - #4654

Open
mcgov (mcgov) wants to merge 3 commits into
mainfrom
mcgov/stack-1-process-wait-line
Open

Process: add wait_line and fix delta_only buffer offset#4654
mcgov (mcgov) wants to merge 3 commits into
mainfrom
mcgov/stack-1-process-wait-line

Conversation

@mcgov

Copy link
Copy Markdown
Collaborator

Part 1 of 9 of a stacked series that reworks the DPDK SRIOV hot plug tests. This one only touches lisa/util/process.py.

  • Add Process.wait_line, which consumes captured output line by line and hands each complete line to a caller supplied predicate, so tests can match on structured content (for example all key=value pairs of a uevent line) instead of the single substring search wait_output does.
  • Fix wait_output's delta_only bookkeeping. The offset was advanced to the end of the whole buffer on a match, so any output that arrived after the matched keyword was skipped by the next call. It now advances to the end of the matched keyword, and on timeout uses the length of the last buffer that was actually searched.

Key Test Cases:
verify_dpdk_sriov_rescind_failover_send_only|verify_dpdk_build_netvsc|smoke_test

Impacted LISA Features:
Sriov, NetworkInterface, SerialConsole

Tested Azure Marketplace Images:

  • canonical 0001-com-ubuntu-server-jammy 22_04-lts latest

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds a line-oriented waiting helper to Process and adjusts wait_output(delta_only=...) offset bookkeeping to better support incremental consumption of process output (used by the DPDK SRIOV hot-plug test rework).

Changes:

  • Added Process.wait_line(...) to consume captured output by complete lines and evaluate a caller-supplied predicate per line.
  • Updated Process.wait_output(...) delta_only offset advancement to move to the end of the matched keyword (and track the last searched buffer length for timeouts).

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread lisa/util/process.py
Comment thread lisa/util/process.py
Comment thread lisa/util/process.py Outdated
@LiliDeng

Copy link
Copy Markdown
Collaborator

AI Test Case Selection

Selected 1 test case(s): smoke_test

Marketplace image: canonical 0001-com-ubuntu-server-jammy 22_04-lts-gen2 latest

Result: Succeeded

View full logs in Azure DevOps

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

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

Suppressed comments (2)

lisa/util/process.py:646

  • wait_output calls self.log_buffer.getvalue() twice per loop iteration (once for find() and once for len()). If new output is appended between those calls, last_search_len can become longer than the buffer that was actually searched, so on timeout log_buffer_offset may skip over output that was never searched (reintroducing the delta_only bug this PR is trying to fix). Capture the buffer once and use it for both operations.
            find_pos = self.log_buffer_offset if delta_only else 0
            buffer = self.log_buffer.getvalue()
            found_at_index = buffer.find(keyword, find_pos)

lisa/util/process.py:611

  • wait_line only evaluates complete newline-terminated lines. If the process exits without emitting a trailing newline, any remaining output after the last \n is never checked against the predicate, so a valid final line can be missed. Consider treating the remaining buffered text as a final line once the process is no longer running.
            if not is_running:
                # the process ended, and its remaining output is consumed.
                break

Copilot AI review requested due to automatic review settings August 12, 2026 20:47

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

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

@LiliDeng

Copy link
Copy Markdown
Collaborator

AI Test Case Selection

Selected 1 test case(s): smoke_test

Marketplace image: canonical 0001-com-ubuntu-server-jammy 22_04-lts-gen2 latest

Result: Succeeded

View full logs in Azure DevOps

Copilot AI review requested due to automatic review settings August 17, 2026 18:05
@mcgov
mcgov (mcgov) force-pushed the mcgov/stack-1-process-wait-line branch from ed1d741 to 900ceb2 Compare August 17, 2026 18:05

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

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

Suppressed comments (1)

lisa/util/process.py:611

  • Major: wait_line() only evaluates lines that end with "\n". If the process exits with a final line that does not include a trailing newline (common for some commands), that last line is never passed to predicate() and will be missed even though it is present in the captured output. Consider treating the trailing partial line as the final line when the process is no longer running.
            if not is_running:
                # the process ended, and its remaining output is consumed.
                break

mcgov (mcgov) and others added 3 commits August 19, 2026 11:47
Add Process.wait_line, which consumes captured output line by line and
hands each complete line to a caller supplied predicate. This allows
matching on structured content (for example all key=value pairs of a
uevent line) instead of the single substring search wait_output does.

Also fix wait_output's delta_only bookkeeping: the offset was advanced
to the end of the buffer on a match, so output that arrived after the
matched keyword was skipped by the next call. Advance it to the end of
the matched keyword instead, and on timeout use the length of the last
buffer that was actually searched.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 5d5f58ad-b9df-4420-ad37-22caee78e925
Add log line change for stdout -> 'output'

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: mcgov <6216084+mcgov@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 19, 2026 18:47
@mcgov
mcgov (mcgov) force-pushed the mcgov/stack-1-process-wait-line branch from 900ceb2 to b6eb640 Compare August 19, 2026 18:47

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

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

Suppressed comments (3)

lisa/util/process.py:613

  • wait_line() only processes newline-terminated lines. If the process exits (or output ends) with a final line that lacks a trailing \n, the comment says the remaining output is consumed but that tail is never passed to predicate, which can cause false timeouts. Consider treating the remaining tail as the final line when the process has ended, and advance log_buffer_offset to the end of the buffer so subsequent calls don’t re-process it.
            if not is_running:
                # the process ended, and its remaining output is consumed.
                break
            if time.time() - start_time >= timeout:
                break

lisa/util/process.py:602

  • When delta_only=True, position starts from log_buffer_offset, but log_buffer_offset may point into the middle of a line (e.g. after a prior wait_output() match). In that case, this loop can hand a truncated line fragment (from the middle to the next \n) to predicate, even though the docstring says it waits for a complete output line. Consider skipping forward to the next newline when starting mid-line so only full lines are evaluated.
            buffer = self.log_buffer.getvalue()
            while True:
                line_end = buffer.find("\n", position)
                if line_end < 0:
                    break

lisa/util/process.py:636

  • last_search_len is initialized to 0, so if the loop body doesn’t execute (e.g. timeout<=0), the timeout path will reset log_buffer_offset to 0 rather than preserving the prior behavior of advancing to the end of the current buffer. Initializing last_search_len from the current buffer length avoids regressing this edge case.
        last_search_len = 0
        start_time = time.time()

@LiliDeng

Copy link
Copy Markdown
Collaborator

AI Test Case Selection

Selected 1 test case(s): smoke_test

Marketplace image: canonical 0001-com-ubuntu-server-jammy 22_04-lts-gen2 latest

Result: Succeeded

View full logs in Azure DevOps

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants