topotato: test_bgp_default_originate_timer.py#126
Draft
Chromico wants to merge 1 commit intoopensourcerouting:topotato-basefrom
Draft
topotato: test_bgp_default_originate_timer.py#126Chromico wants to merge 1 commit intoopensourcerouting:topotato-basefrom
Chromico wants to merge 1 commit intoopensourcerouting:topotato-basefrom
Conversation
eqvinox
reviewed
Sep 8, 2023
test_bgp_default_originate_timer.py
Outdated
| "bgpd", | ||
| f"show bgp ipv4 unicast 0.0.0.0/0 json", | ||
| maxwait=10.0, | ||
| compare=expected, |
There was a problem hiding this comment.
in the original topotest this is a "negative" check, it checks the route does NOT exist — need to do something similar here.
Good approach would be:
- wait for some other route to check the BGP session is up
- then check default route is empty/absent
test_bgp_default_originate_timer.py
Outdated
| """ | ||
| configure terminal | ||
| router bgp | ||
| bgp default-originate timer 10 |
test_bgp_default_originate_timer.py
Outdated
| r2, | ||
| "bgpd", | ||
| f"show bgp ipv4 unicast 0.0.0.0/0 json", | ||
| maxwait=10.0, |
There was a problem hiding this comment.
… so this wait is not long enough for the 10s above.
⇒ shorten the 10s above, and make this down here maybe 15s (there is some additional delay, probably from internal BGP timers) ⇒ then it should work
test_bgp_default_originate_timer.py
Outdated
| compare="", | ||
| ) | ||
|
|
||
| yield from AssertVtysh.make( |
test_bgp_default_originate_timer.py
Outdated
| r3, | ||
| "vtysh", | ||
| """ | ||
| configure terminal |
There was a problem hiding this comment.
remove configure terminal for ReconfigureFRR
eqvinox
reviewed
Sep 8, 2023
test_bgp_default_originate_timer.py
Outdated
|
|
||
| yield from AssertVtysh.make( | ||
| r3, | ||
| "vtysh", |
There was a problem hiding this comment.
Using bgpd here makes the test a little bit faster (or use less CPU)
Chromico
changed the title
Signed-off-by: Nathan Mangar <nathan@thundergear.io>
Chromico
force-pushed
the
bgp-default-originate-timer
branch
from
6138071 to
a9cf782
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The test is not consistently passing. Does not seem to be an issue with the timing.