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

Make sure chip-all-clusters-app is ready before we try to do PASE setup with it. #7561

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .github/workflows/tests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,8 @@ jobs:
with:
submodules: true
- name: Setup Environment
run: brew install openssl pkg-config
# coreutils for stdbuf
run: brew install openssl pkg-config coreutils
- name: Try to ensure the directories for core dumping and diagnostic log collection exist and we can write them.
run: |
sudo chown ${USER} /cores || true
Expand Down
34 changes: 32 additions & 2 deletions scripts/tests/test_suites.sh
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,38 @@ for j in "${iter_array[@]}"; do
echo " ===== Running test: $i"
echo " * Starting cluster server"
rm -rf /tmp/chip_tool_config.ini
out/debug/chip-all-clusters-app &
background_pid=$!
# This part is a little complicated. We want to
# 1) Start chip-all-clusters-app in the background
# 2) Pipe its output through tee so we can wait until it's ready for a
# PASE handshake.
# 3) Save its pid off so we can kill it.
#
# The subshell with echoing of $! to a file descriptor and
# then reading things out of there accomplishes item 3;
# otherwise $! would be the last-started command which would
# be the tee. This part comes from https://stackoverflow.com/a/3786955
# and better ideas are welcome.
#
# The stdbuf -o0 is to make sure our output is flushed through
# tee expeditiously; otherwise it will buffer things up and we
# will never see the string we want.

# Clear out our temp files so we don't accidentally do a stale
# read from them before we write to them.
rm -rf /tmp/all-clusters-log
rm -rf /tmp/pid
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this do a 'sync' or a 'flush' to ensure the deletion actually happened?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This does not seem relevant in that we don't care what the on-disk state is, just what happens when we open the filename, and that part should be all set once rm returns. In particular, I was running into issues where we were seeing a "Server Listening" still in the all-clusters-log from a previous iteration once I moved reading that to before reading /tmp/pid..... But if we rm that should not happen, as far as I know.

(
stdbuf -o0 out/debug/chip-all-clusters-app &
echo $! >&3
) 3>/tmp/pid | tee /tmp/all-clusters-log &
while ! grep -q "Server Listening" /tmp/all-clusters-log; do
:
done
# Now read $background_pid from /tmp/pid; presumably it's
Copy link
Contributor

Choose a reason for hiding this comment

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

The term 'presumably' doesn't inspire confidence...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, it does not! We are racing the I/O redirection to /tmp/pid in the background shell against the all-clusters-app/tee combo that is writing to /tmp/all-clusters-log. I would expect the former to win the race and put the PID in the right place before we see "Server Listening" in pretty much all cases, but there's no guarantee here. :(

We need to figure out a better setup for this....

# landed there by now. If we try to read it immediately after
# kicking off the subshell, sometimes we try to do it before
# the data is there yet.
background_pid="$(</tmp/pid)"
echo " * Pairing to device"
out/debug/standalone/chip-tool pairing onnetwork 1 20202021 3840 ::1 11097
echo " * Starting test run: $i"
Expand Down