Skip to content

Update run_process() implementation for udstool tests#467

Open
nemadhu wants to merge 1 commit into
eclipse-openbsw:mainfrom
esrlabs:cr-612008
Open

Update run_process() implementation for udstool tests#467
nemadhu wants to merge 1 commit into
eclipse-openbsw:mainfrom
esrlabs:cr-612008

Conversation

@nemadhu
Copy link
Copy Markdown
Contributor

@nemadhu nemadhu commented May 29, 2026

Update run_process() implementation for udstool tests

Adding run_process() helper using python subprocess
The helper function used to run udstool for test/pyTest/uds/test_udsToolRDBI.py took longer than the one second wait as per the previous implementation and the test was failing. Replacing the implementation of run_process() so as to fix this issue.

@nemadhu nemadhu changed the title Add run_process() fixture in conftest Updated run_process() implementation for udstool tests May 29, 2026
@nemadhu nemadhu changed the title Updated run_process() implementation for udstool tests Update run_process() implementation for udstool tests May 29, 2026
Copy link
Copy Markdown
Contributor

@christophruethingbmw christophruethingbmw 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 to me

Comment thread test/pyTest/uds/test_udsToolRDBI.py Outdated
)
print(command)
output = helper.run_process(command)
output = run_process(command, "PositiveResponse")
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.

Should we specify the timeout explicitly here? Otherwise we use the default (of I think 5s) and we are actually not aware what timeout we will have here?

Copy link
Copy Markdown
Contributor

@DominikAFischer DominikAFischer left a comment

Choose a reason for hiding this comment

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

SOme remarks

def run_process(cmd, output_contains, timeout=5):
"""Run a shell command and wait until specific output appears.

The command is executed in ``/bin/sh`` using ``subprocess``. The function waits
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.

/bin/sh vs /bin/bash below

`` output_contains``.
"""
proc = subprocess.Popen(
["/bin/bash", "-lc", cmd],
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.

I would use a non-login shell here to isolate: ["/bin/bash", "-c", cmd],

poller.register(proc.stdout, select.POLLIN)

output = []
deadline = time.time() + timeout
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.

Nit: you could use deadline = time.monotonic() + timeout here, then

    try:
        while True:
            remaining = deadline - time.monotonic()
            if remaining <= 0:
               # raise timeout errro

remaining_ms = max(0, int((deadline - time.time()) * 1000))
events = poller.poll(remaining_ms)
if not events:
break
Copy link
Copy Markdown
Contributor

@DominikAFischer DominikAFischer May 29, 2026

Choose a reason for hiding this comment

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

This needs to be a continue, no? Otherwise we would quit in the first iteration if we haven't received any events yet

if not events:
break
chunk = os.read(proc.stdout.fileno(), 4096).decode()
if not chunk:
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.

Do we need to check if the process has finished?

if not chunk:
   if proc.poll() is not None:
      break

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

proc.poll() check is added after poller.poll()

raise AssertionError(f"[TIMEOUT] Did not find {output_contains} within {timeout}s\n Output:\n{''.join(output)}")
finally:
proc.kill()
proc.wait() No newline at end of file
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.

missing empty line at EOF

Comment thread test/pyTest/requirements.txt Outdated
typing_extensions==4.12.2
udsoncan==1.23.1
wrapt==1.16.0
pexpect==4.9.0
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.

Revert the addition of pexpect

poetry-core==2.2.1
# via poetry
ptyprocess==0.7.0
# via pexpect
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.

restore the old lockfile to revert the addition of pexpect

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.

3 participants