napalm-automation / napalm-base

Apache License 2.0
32 stars 48 forks source link

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

Open manuel-domke opened 7 years ago

manuel-domke commented 7 years ago

hi,

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

dbarrosop commented 7 years ago

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?

manuel-domke commented 7 years ago

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 commented 7 years ago

See: https://github.com/napalm-automation/napalm-base/issues/238

mirceaulinic commented 7 years ago

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 commented 7 years ago

Response here: https://github.com/napalm-automation/napalm-base/issues/238#issuecomment-296098915