Skip to content

Conversation

@wenovus
Copy link
Contributor

@wenovus wenovus commented Sep 1, 2023

  • policytest is adapted from local_test's policy_test.go and is suitable for being used in Ondatra tests instead of locally.

Previous PR: #253

I suspect the reason for the "update is stale" is due to a race
condition between different calls to gNMI.Set that each may provide a
different unix.Now() value depending on when it is executed prior to
updating the cache. I believe the correct fix is to actually to ignore
it.

WIP -- isn't Set synchronous? If so how could this happen?

clue: is it always happening on default values?
@wenovus wenovus changed the base branch from refactor-bgp to fix-bgp-reset September 1, 2023 17:17
@wenovus wenovus requested a review from DanG100 September 1, 2023 17:19
@DanG100
Copy link
Contributor

DanG100 commented Sep 1, 2023

What's the long term plan for this? It seems like there's a lot duplication (code and test behavior) of the local test.

Base automatically changed from fix-bgp-reset to main September 1, 2023 21:48
@wenovus
Copy link
Contributor Author

wenovus commented Sep 1, 2023

What's the long term plan for this? It seems like there's a lot duplication (code and test behavior) of the local test.

I agree there is a lot of duplication. I think an Ondatra binding or generics/interfaces may be the solution to deal with the different Device/Port/ygnmi API type differences, as otherwise the two are identical.

My judgement is that local_tests will be useful as a lightweight way to test and debug BGP policy development, so once the Ondatra version of the test is stable, I'll look at an Ondatra bind for local lemmings.

@wenovus
Copy link
Contributor Author

wenovus commented Sep 6, 2023

@DanG100 let me know if you have more comments on this one. This PR contains the changes that reduces test flakiness due to chaining the PRs.

@wenovus wenovus merged commit bcd2dc3 into main Sep 6, 2023
@wenovus wenovus deleted the ipv4-prefix-set-integration branch September 6, 2023 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants