Skip to content
This repository has been archived by the owner on Sep 17, 2019. It is now read-only.

Test case for static routes (v4+v6) #233

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

Test case for static routes (v4+v6) #233

wants to merge 2 commits into from

Conversation

manuel-domke
Copy link

@manuel-domke manuel-domke commented Apr 4, 2017

hi,

I need test cases for static ipv4/ipv6 routes for my changes made in napalm-iosxr.

@dbarrosop
Copy link
Member

Ok, adding these tests basically break CIs for other drivers so we will have to be careful with this one.

Drivers we have to fix:

  1. EOS
  2. IOSXR
  3. Junos
  4. Panos

Shouldn't be terrible.

However, I was wondering if we should just remove the "protocol" from test_get_route_to so we can control with the test cases (default and the rest) the data we get and parse instead.

@manuel-domke @mirceaulinic any thoughts?

@dbarrosop dbarrosop mentioned this pull request Apr 6, 2017
3 tasks
@manuel-domke
Copy link
Author

I was wondering on how many of the other implementations I'll break. Glad its just three :)
And - yes. Lets remove the protocol for "default" test cases.

@dbarrosop
Copy link
Member

See: #238

@mirceaulinic
Copy link
Member

mirceaulinic commented Apr 20, 2017

I need to think more about it, but for the moment I'm not sure I can agree with this: do we want to have more bespoken methods and remove an useful functionality just for the sake of testing an argument? If the protocol is too problematic, we could just ignore it, as we do for other methods, e.g.: get_bgp_neighbors_detail or others having optional arguments. What would we do it we'd need to test the argument neighbor for get_bgp_neighbors_detail? Would we implement get_bgp_neighbors_detail_neighbor?
Same for ping: why not just implement ping_vrf, ping_ttl etc. methods, to be easier testing? :)

@dbarrosop
Copy link
Member

Response here: #238 (comment)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants