WAR-1255: response stored in data repository through response_ref get stored with control characters by navvrnd · Pull Request #72 · warriorframework/warriorframework

navvrnd · 2017-07-14T05:38:10Z

response from cli commands stored through response_ref and pattern gets stored with control characters which is causing issues while retrieving later. This was tested with the local team facing the issue and was working fine. For testing, one can take a test case which will store data with control characters in data repository and then retrieve later without any issues.

…sitory corrected

…framework into bugfix/WAR-1255

kaheichan · 2017-07-17T16:03:53Z

@navvrnd Please provide title and description for this one, #73, #74, #75

Annam16 · 2017-07-27T12:57:54Z

@navvrnd can you add a detailed description in the JIRA ticket as well as how to test this fix in pull request description?

Annam16 · 2017-07-31T05:14:21Z

@navvrnd Can you add sample test case to test this fix in JIRA

Annam16

Please add sample test case to test this fix in JIRA

Annam16 · 2017-08-01T04:50:28Z

@navvrnd Can you please add the below details in JIRA ticket.

failed test case and it's related files.
Failure log before this fix.
Pass log after this fix.
How you conclude that the test case failed due to control characters because in ticket description it is mentioned that "not able to retrieve it in another test step"

venkat1077 · 2017-08-07T12:25:03Z

+            reobj = re.search(resp_pat_req, response)
+            response = reobj.group(0) if reobj is not None else ""
+            # removing non-ascii/control characters (line feed) from response
+            response = "".join([c for c in response if 31 < ord(c) < 127])


Can you check if you can use "rem_nonprintable_ctrl_chars" method from testcase_utils_class file for removing ascii control chars.

The function "rem_nonprintable_ctrl_chars" only removes a subset of control characters. I wanted to make it generic. May be we can replace that with this code, but I don't what might break. Please do comment.

'rem_nonprintable_ctrl_chars' method removes chars with hex values other than 'x20-x7E, x09 & x0A' which is equivalent to accepting decimal values: 32-126, 9(horizontal tab) & 10(new line). I see the difference is only 2 chars(horizontal tab & new line) which we should allow IMO.

Annam16

The attached failure log in JIRA clearly says that, while converting a particular value to the data type "int", exception thrown. And this exception has to be handled in "Framework/Utils/data_Utils.py" "convert2type" method. So kindly add the changes in "convert2type" method.

Annam16 · 2017-08-09T06:06:21Z

+    try:
+        cvalue = convert(value)
+    except Exception as exception:
+        print_exception(exception)


It would be better if we add the warning message instead of printing the whole exception.

I see couple of exceptions , one is we are raising and the other one is because of returning "None". Do necessary changes.

Multiple return statements inside single method. we can assign it to a variable called "status" and handle that value in all the methods where this method is been called already.

navvrnd · 2017-08-09T10:34:59Z

made the changes requested. Please review.

…framework into bugfix/WAR-1255

navvrnd · 2017-08-14T06:36:29Z

cli full regression log attached in the jira ticket. The following tests failed in the full regression which does not look like issues due to this change:

connect: connect_call_connect_telnet_multi_subsys, connect_call_connect_telnet_custom_keystroke, connect_abort_as_error
cli_global_attr: tc_global_title_no_subsys, tc_global_title_one_subsys, tc_global_title_multi_subsys
cli_local_attr: tc_local_title_no_subsys
cli_other_attr: tc_cmd_retry_attrib
log_verify_on_suite: log_verify_on

…framework into bugfix/WAR-1255

Annam16

Approving this pull request by testing it with "warrior_fnc_tests/warrior_tests/testcases/cli_tests/other_attr/tc_testdata_relations_verify.xml" by passing different inputs. Working fine.

kaheichan

Questions are raised in Jira ticket

kaheichan · 2017-10-04T16:55:12Z

Hi @navvrnd
More comment has been added in the Jira ticket
the TODO is:

Have a better error message when substitution went wrong
Add a testcase to test this error

Hi @kaheichan
addressed the first comment. For addressing the second comment, can you please let me know, how to create data with non-printable characters? The system where the issue was found is product team's system, which can not be used by us. And in our systems, this issue has not been reproduced. I am assuming that you need a test case to catch the non-printable characters issue if it does occur. Please let me know your comments.

10/5 Ka
Hi @navvrnd , you can use echo or touch in terminal to make file with non-printable content
https://stackoverflow.com/questions/35947715/how-can-i-create-a-file-with-a-non-printing-character-in-the-name
another way is using the function that you called in your fix, the
rem_nonprintable_ctrl_chars()
loop through the characters that need to be replaced in the replace function inside
and write those to a file in python

…framework into bugfix/WAR-1255

kaheichan · 2017-10-10T16:45:42Z

Upon further investigation in the dummy-td file and the attached td file, I found out that in the dummy-td file
user specifies a regular expression resp_pat_req="[\d]+\s" to capture the response from server.
\s in the resp_pat_req captures the non-printable character “\n” in the response “2017041011\n”
When the user uses the response as an integer conditional value, Python tries to cast “2017041011\n” to an int, which fails because \n cannot be converted into integer.

This is a user input error that we cannot prevent.
What we can do is when we are trying to use the saved value as conditional value and converting it into integer, we should remove the non-printable character before the conversion.

The current fix in WAR-1255 will cause issues in the future when the user actually wants to capture non-printable character such as newline.
Since the fix removes all non-printable characters, a scenario where user wants to capture a multi-line response will be distorted because we remove the new-line character in this fix.
Please update the fix and let us know immediately so we can review and merge asap.

…framework into bugfix/WAR-1255

…ta repository

navvrnd · 2017-10-11T12:18:06Z

Hi @kaheichan addressed all the comments. Please check and approve.

10/12 -Ka
@navvrnd hello, have some small comments, please address. Code logic looks clear

10/13 - Arvind
@kaheichan done. Please check.

kaheichan · 2017-10-12T19:02:20Z

                    except SyntaxError:
-                        print_error("Syntax Error - " +
-                                    error_msg2.format(string, value))
+                        from WarriorCore.Classes.testcase_utils_class import TestcaseUtils


Please put import statement at the beginning of the file

kaheichan · 2017-10-12T19:05:43Z

-                                    error_msg2.format(string, value))
+                        from WarriorCore.Classes.testcase_utils_class import TestcaseUtils
+                        tuc_obj = TestcaseUtils()
+                        print_info("Checking after removing the illegal characters")


Please provide a more detailed message
"Cannot convert value: {} into correct format, removing non-printable characters, will attempt conversion again"

Changed to "Cannot convert below value into correct format, removing non-printable characters, will attempt conversion again" and the actual value in next line, since it mangles the other characters in the string.

kaheichan · 2017-10-13T22:10:47Z

Hi @navvrnd
The code looks good, I run the new testcase and verified it works
however cli_proj is failing and it shouldn't
please refer to the internal PR for more info

Hi @kaheichan
After the merge the regression passed except for tc_multiple_response_patterns.xml since its code base war-1156 is still yet to be merged.

@navvrnd war-1156 was merged. Please rerun regression and let us know it passed.

@kaheichan reran and attached in the ticket. Looks like the code for WAR-1439 is still not yet in which is causing one failure.

10/23 -Ka
Hi @navvrnd
I verified the test again and found out that the testcase doesn't actually found the response pattern in the response. I have added a warning message when it is not found. I have also changed the sample response in the server to actually test the \s part
We still need to discuss the question sent in the email

…framework into bugfix/WAR-1255

navvrnd · 2017-10-24T13:46:16Z

Hi @kaheichan I have attached all pass full regression log in the ticket after commenting out list_range test case which was causing hang. Please let me know, what else to be done for this ticket.

…framework into bugfix/WAR-1255

kaheichan · 2017-10-30T20:17:17Z

Saw the change and verified the fix, thank you @navvrnd

Arvind Kumar Sethuraman and others added 2 commits June 28, 2017 16:26

bugfix/WAR-1255: response stored with control characters in data repo…

7766f45

…sitory corrected

Merge branch 'develop' of https://github.com/warriorframework/warrior…

8d5f68e

…framework into bugfix/WAR-1255

navvrnd requested review from Annam16, bjrr, satyakrish and venkat1077 July 14, 2017 05:40

navvrnd self-assigned this Jul 14, 2017

navvrnd changed the title ~~Bugfix/war 1255~~ Bugfix/war 1255: response stored in data repository through response_ref get stored with control characters Jul 18, 2017

Annam16 suggested changes Jul 31, 2017

View reviewed changes

venkat1077 reviewed Aug 7, 2017

View reviewed changes

Annam16 reviewed Aug 8, 2017

View reviewed changes

incorporated review comments

4b90be2

Annam16 previously approved these changes Aug 8, 2017

View reviewed changes

Annam16 reviewed Aug 9, 2017

View reviewed changes

incorporated review comments

5903b04

navvrnd dismissed Annam16’s stale review via 5903b04 August 9, 2017 10:30

Arvind Kumar Sethuraman added 2 commits August 9, 2017 16:28

Merge branch 'develop' of https://github.com/warriorframework/warrior…

04c0f14

…framework into bugfix/WAR-1255

Merge branch 'develop' of https://github.com/warriorframework/warrior…

40f348f

…framework into bugfix/WAR-1255

Merge branch 'develop' of https://github.com/warriorframework/warrior…

d473c2b

…framework into bugfix/WAR-1255

Annam16 previously approved these changes Aug 17, 2017

View reviewed changes

venkat1077 previously approved these changes Aug 17, 2017

View reviewed changes

kaheichan requested review from kaheichan and removed request for satyakrish September 26, 2017 18:32

kaheichan suggested changes Sep 26, 2017

View reviewed changes

navvrnd dismissed Annam16’s stale review via 90a8f23 October 4, 2017 13:35

Arvind Kumar Sethuraman added 3 commits October 5, 2017 11:19

removed unnecessary imports

388d9a9

Merge branch 'develop' of https://github.com/warriorframework/warrior…

d0ee2f6

…framework into bugfix/WAR-1255

better error message when failure occurs

7d94fda

Arvind Kumar Sethuraman added 5 commits October 11, 2017 14:23

Merge branch 'develop' of https://github.com/warriorframework/warrior…

9c14cc5

…framework into bugfix/WAR-1255

removing non printable chars is moved to when it is evaluated from da…

b47291e

…ta repository

removed unnecessary import

4c03a3c

fixed travis issues

b02b283

added a message to be printed

162ffe0

kaheichan reviewed Oct 12, 2017

View reviewed changes

changing print message to more readable

b853870

Arvind Kumar Sethuraman and others added 5 commits October 16, 2017 16:44

Merge branch 'develop' of https://github.com/warriorframework/warrior…

b09189a

…framework into bugfix/WAR-1255

resolved merge conflicts

7e2f162

fixed an issue in multiple responses

60febed

add warning message when response pattern not found

33e53da

edit print message

6651e50

bjrr changed the title ~~Bugfix/war 1255: response stored in data repository through response_ref get stored with control characters~~ WAR-1255: response stored in data repository through response_ref get stored with control characters Oct 24, 2017

Merge branch 'develop' of https://github.com/warriorframework/warrior…

fae9700

…framework into bugfix/WAR-1255

Arvind Kumar Sethuraman added 2 commits October 27, 2017 16:41

Merge branch 'develop' of https://github.com/warriorframework/warrior…

b0d4153

…framework into bugfix/WAR-1255

removed WAR-1495 fix from here

a5475dc

kaheichan approved these changes Oct 30, 2017

View reviewed changes

bjrr merged commit e15616c into develop Oct 30, 2017

bjrr added this to the Warrior-3.5.0 milestone Nov 1, 2017

Uh oh!

Conversation

navvrnd commented Jul 14, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kaheichan commented Jul 17, 2017

Uh oh!

Annam16 commented Jul 27, 2017

Uh oh!

Annam16 commented Jul 31, 2017

Uh oh!

Annam16 left a comment

Choose a reason for hiding this comment

Uh oh!

Annam16 commented Aug 1, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Annam16 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

navvrnd commented Aug 9, 2017

Uh oh!

navvrnd commented Aug 14, 2017

Uh oh!

Annam16 left a comment

Choose a reason for hiding this comment

Uh oh!

kaheichan left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kaheichan commented Oct 4, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kaheichan commented Oct 10, 2017

Uh oh!

navvrnd commented Oct 11, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kaheichan commented Oct 13, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

navvrnd commented Oct 24, 2017

Uh oh!

kaheichan commented Oct 30, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

navvrnd commented Jul 14, 2017 •

edited

Loading

kaheichan left a comment •

edited

Loading

kaheichan commented Oct 4, 2017 •

edited

Loading

navvrnd commented Oct 11, 2017 •

edited

Loading

kaheichan commented Oct 13, 2017 •

edited

Loading