On 11/19/24 19:54, Daniel P. Berrangé wrote:
+ if success is None or success in msg:
As an optimization, you could use msg.endswith(success) and
msg.endswith(failure), which would avoid the most blatant cases of O(n^2)
behavior.
More important, I think "if success is None" should not be here, because it
will exit after one char. Instead...
+ done = True
+ break
+ if failure and failure in msg:
+ done = True
+ vm.console_socket.close()
+ test.fail(
+ f"'{failure}' found in console, expected '{success}'")
+
+ if c == b'\n':
Here you can put
done = success is None
Hmmm, this can only be a problem if "success" is None, and
"failure" is not None, and although the old code would
technically work in that case, I think it is actually an
unknown/invalid usage scenario.
If BOTH "success" and "failure" are None, this method won't
be called at all. It is valid for "failure" to be none, but
I don't think it makes semantic sense for "success" to also
be None, while have "failure" be non-None.
"Read a line and check that it doesn't contain a substring" seemed like
a plausible thing to test for, but you're right it doesn't make much
sense in this context. It would make more sense if _console_readline
returned the full line, at which point it would be a completely
different function and probably not underscore-prefixed.
So yeah, this is okay:
assert send_string is not None or success_message is not None
whether done in the caller or in _console_readline itself, as you prefer.
Paolo
