-
Notifications
You must be signed in to change notification settings - Fork 760
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
Add a testcase for GNOI OS.Verify. #16770
base: master
Are you sure you want to change the base?
Conversation
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
match = json_pattern.search(output) | ||
if match: | ||
try: | ||
return json.loads(match.group()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's fragile to use regex to extract a piece of json string. How about just skip first line and parse 2nd line as json string?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't that less tolerant to different outputs? Say someone would print:
System Time: {response json}
Or simply
{response json}
then the method won't work. The original method works as long as the first json substring is the response json, doesn't matter what you print before it (as long as it doesn't somehow contain another json).
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
Description of PR
Summary:
Add a testcase for GNOI OS.Verify.
Fixes # (issue)
Type of change
Back port request
Approach
What is the motivation for this PR?
Add a testcase for GNOI OS.Verify.
How did you do it?
Add a testcase for GNOI OS.Verify.
How did you verify/test it?
On physical switch:
with an up to date image
Any platform specific information?
Supported testbed topology if it's a new test case?
Documentation