Since commit 64859fc, we block on startup waiting for a reply to our terminal queries.
One of these is the "Primary Device Attribute" query, which is reasonably widely supported, which is why we haven't heard of reports of hangs yet.
But it does not have to be supported, and the failure mode is that fish just hangs indefinitely.
This should have a timeout.
To test, try:
diff --git a/tests/pexpect_helper.py b/tests/pexpect_helper.py
index 85c0bab6c..40347926b 100644
--- a/tests/pexpect_helper.py
+++ b/tests/pexpect_helper.py
@@ -175,8 +175,8 @@ class SpawnedProc(object):
)
self.spawn.delaybeforesend = None
self.prompt_counter = 0
- if env.get("TERM") != "dumb":
- self.spawn.send('\x1b[?123c') # Primary Device Attribute
+ # if env.get("TERM") != "dumb":
+ # self.spawn.send('\x1b[?123c') # Primary Device Attribute
def time_since_first_message(self):
"""Return a delta in seconds since the first message, or 0 if this is the first."""
diff --git a/tests/pexpects/set_color.py b/tests/pexpects/set_color.py
index 2534ef3c9..2cbd505b3 100644
--- a/tests/pexpects/set_color.py
+++ b/tests/pexpects/set_color.py
@@ -19,7 +19,7 @@ expect_prompt()
# See that --print-colors prints the colors colored.
# Note that we don't check *all* of them, just a few.
sendline("set_color --print-colors")
-expect_str("black")
+expect_str("black", timeout=120)
expect_str("blue")
expect_str("brblack")
expect_str("brblue")This will hang for two minutes before giving up on the pexpect side. That's because pexpect, by default, does not reply to the device attribute query.
So we already know one case that will break.
Since commit 64859fc, we block on startup waiting for a reply to our terminal queries.
One of these is the "Primary Device Attribute" query, which is reasonably widely supported, which is why we haven't heard of reports of hangs yet.
But it does not have to be supported, and the failure mode is that fish just hangs indefinitely.
This should have a timeout.
To test, try:
This will hang for two minutes before giving up on the pexpect side. That's because pexpect, by default, does not reply to the device attribute query.
So we already know one case that will break.