Fix random failures in unit tests #371
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #371 +/- ##
==========================================
+ Coverage 28.12% 28.13% +0.01%
==========================================
Files 128 128
Lines 11207 11209 +2
==========================================
+ Hits 3152 3154 +2
Misses 7773 7773
Partials 282 282
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| if _, ok := b.LoadRequests[id]; !ok { | ||
| break | ||
| } | ||
| } |
There was a problem hiding this comment.
can u avoid infinite loop and just have maxRetries and fail if its reached ?
There was a problem hiding this comment.
I figured someone wouldn't like the infinite loop :-). How about this new solution that doesn't use random numbers?
Failures were caused when the same random id was used for more than one program in the fake bpfman client. Changed the code to use a deterministic incrementing ID. Signed-off-by: Andre Fredette <[email protected]>
anfredette
force-pushed
the
fix-bpfman-fake-client
branch
from
3573998 to
cffb2b8
Compare
| func (b *BpfmanClientFake) Load(ctx context.Context, in *gobpfman.LoadRequest, opts ...grpc.CallOption) (*gobpfman.LoadResponse, error) { | ||
| id := rand.Intn(100) | ||
| currentID++ | ||
| id := currentID |
There was a problem hiding this comment.
do u still need currentID ?
There was a problem hiding this comment.
Do you have a different solution in mind?
There was a problem hiding this comment.
BTW, I've seen this issue for a long time. I.e., unit tests would fail sometimes, but when you run them again they would pass. It just started happening often enough when I started using more programs that it bothered me enough to figure out why.
There was a problem hiding this comment.
i was just thinking of adding a cntr of 100 or 100 retries to get unused id from random instead of infinite loop
There was a problem hiding this comment.
I understood what you were suggesting, and I think it would work, but I think this is better -- it's simpler and always unique because you remember the last ID and increment it for the next one.
I thought about doing it this way after I made the first change, but figured the previous way was good enough and didn't bother going back to change it.
There was a problem hiding this comment.
sure I am fine either way was just answering ur question
|
/LGTM |
Failures were caused when the same randomly generated id was used for more than one program
in the fake bpfman client. Changed the code to use a deterministic incrementing ID.