Filter Corstone's stdout #61
Conversation
Corstone prints out some informational text to stdout, but this text conflicts with expectations from tests about the contents of stdout. This patch filters out this text to send to stdout only what matters.
| # failure. To work around this, we cut out the model's boilerplate output. | ||
| if fvp_model == "corstone-310": | ||
| decoded_stdout = result.stdout.decode() | ||
| relevant_lines = decoded_stdout.splitlines()[5:-2] |
There was a problem hiding this comment.
It makes me nervous that we're just deleting a fixed number of lines here, without any kind of check that they're the text we expect them to be.
I can easily imagine that an updated FVP might change the format of its output, perhaps removing the boilerplate entirely or rewording it so that it takes a different number of lines. It might take us ages to find out why the tests are failing, especially if the person investigating has no idea that this code is here!
Perhaps it would be safer to do some kind of a check that the lines being deleted are what we expect? And if they're not, give a nice clear error message, so that if the FVP output does change in future, we'll immediately know to come to this file and adjust (or remove) our workaround.
There was a problem hiding this comment.
This is very reasonable. I changed the script to check for the expected contents of Corstone's output, and extract the actual stdout from it.
There was a problem hiding this comment.
I'm not really sure if the dates and times should be hardcoded in the regex as they are right now. If you think their actual values don't matter, I can change the regex
There was a problem hiding this comment.
Thanks, that certainly looks safer.
I think I'd probably make the regex general enough to ignore changes in the copyright date and that hex string that looks like a git commit. Then there's at least some chance we don't have to change anything in the next FVP update, but we'll still spot the problem if the boilerplate completely changes its format.
vhscampos
force-pushed
the
filter-corstone-stdout
branch
from
a4a9fdf to
a18c9c0
Compare
statham-arm
left a comment
statham-arm
left a comment
There was a problem hiding this comment.
LGTM this time, thanks!
Corstone prints out some informational text to stdout, but this text conflicts with expectations from tests about the contents of stdout. This patch filters out this text to send to stdout only what matters.
Corstone prints out some informational text to stdout, but this text conflicts with expectations from tests about the contents of stdout. This patch filters out this text to send to stdout only what matters.
2 participants
Corstone prints out some informational text to stdout, but this text conflicts with expectations from tests about the contents of stdout.
This patch filters out this text to send to stdout only what matters.