Fix broken regex and add support for discarded packets in get_interfaces_counters

napalm-automation / napalm-ios

Apache License 2.0

31 stars 40 forks source link

Fix broken regex and add support for discarded packets in get_interfaces_counters #118

Closed jorgenbl closed 7 years ago

jorgenbl commented 7 years ago

The proposed fix will allow for the second match group to be correct (rx_octets and tx_octets) I also added support for discarded/dropped packets with the "show interface status" command.

mirceaulinic commented 7 years ago

Hi @jorgenbl - thanks for this PR!

The tests are not passing (https://travis-ci.org/napalm-automation/napalm-ios/jobs/206914373) as you need to add a new mock file called show_interface_summary.txt under test/unit/mocked_data/test_get_interfaces_counters/normal having the output of show interface summary. Then you should be ready to go!

jorgenbl commented 7 years ago

Hi,

I was testing on gns3 on a 7200 series router, but It seems like you guys are testing on a csr1000v built with vagrant. Will I have to create the same device with vagrant and run the commands to produce the mock file? Can I use the device I have or how does this work? :)

Sorry for the noob questions, but I'm a bit new to this testing procedure.

dbarrosop commented 7 years ago

@jorgenbl mocked data can come from anywhere as long as it's from a modern IOS-XE device, they don't really have to come from the same source.

coveralls commented 7 years ago

Coverage increased (+0.3%) to 66.436% when pulling 9b505213007ae3edaa1250fd1b43b9b3be8c315d on jorgenbl:develop into b123c8b26a663fe4d6b32b92ae70882ae47b8e88 on napalm-automation:develop.

coveralls commented 7 years ago

Coverage increased (+0.3%) to 66.436% when pulling 9b505213007ae3edaa1250fd1b43b9b3be8c315d on jorgenbl:develop into b123c8b26a663fe4d6b32b92ae70882ae47b8e88 on napalm-automation:develop.

ktbyers commented 7 years ago

I added one additional improvement that needs made (documenting the regex) and I had a question that I included in the review.

mirceaulinic commented 7 years ago

Hi @ktbyers - I think the comments are not visible until the review is finished with explicit Approve/Request Changes/Just comment. I've also missed that many times and posted the comments only few weeks after...

ktbyers commented 7 years ago

@mirceaulinic Okay, that comment aspect is annoying...

coveralls commented 7 years ago

Coverage increased (+0.3%) to 66.436% when pulling fca5890ec5ccb851616dd82877f53a8eed7108a1 on jorgenbl:develop into b123c8b26a663fe4d6b32b92ae70882ae47b8e88 on napalm-automation:develop.

ktbyers commented 7 years ago

I will try to merge this in later today.