Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a (currently disabled) regression test for #1341 #2524

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

stevenengler
Copy link
Contributor

@stevenengler stevenengler commented Nov 7, 2022

We should enable this test when we fix #1341. Adding this test now since it was already written and it will probably be a while before we fix the issue.

@stevenengler stevenengler self-assigned this Nov 7, 2022
@stevenengler stevenengler changed the title Added a (currently disabled) regression test for #1341 Add a (currently disabled) regression test for #1341 Nov 7, 2022
@github-actions github-actions bot added Component: Build Build/install tools and dependencies Component: Testing Unit and integration tests and frameworks labels Nov 7, 2022
@codecov
Copy link

codecov bot commented Nov 7, 2022

Codecov Report

Base: 67.65% // Head: 66.91% // Decreases project coverage by -0.73% ⚠️

Coverage data is based on head (8d9178c) compared to base (58b450b).
Patch coverage: 1.66% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2524      +/-   ##
==========================================
- Coverage   67.65%   66.91%   -0.74%     
==========================================
  Files         190      191       +1     
  Lines       28106    28178      +72     
  Branches     5554     5578      +24     
==========================================
- Hits        19014    18856     -158     
- Misses       4668     4938     +270     
+ Partials     4424     4384      -40     
Flag Coverage Δ
tests 66.91% <1.66%> (-0.74%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/test/regression/1341/test_regression_1341.rs 1.66% <1.66%> (ø)
src/main/host/syscall/handler/eventfd.rs 0.00% <0.00%> (-68.58%) ⬇️
src/main/host/descriptor/eventfd.rs 0.00% <0.00%> (-64.65%) ⬇️
src/main/host/syscall/handler/ioctl.rs 24.24% <0.00%> (-27.28%) ⬇️
src/main/host/syscall/handler/unistd.rs 55.79% <0.00%> (-15.46%) ⬇️
src/main/host/syscall/handler/fcntl.rs 50.00% <0.00%> (-11.77%) ⬇️
src/main/host/syscall/handler/time.rs 58.53% <0.00%> (-7.32%) ⬇️
src/main/host/syscall/handler/mod.rs 77.14% <0.00%> (-7.15%) ⬇️
src/main/host/descriptor/socket/mod.rs 42.16% <0.00%> (-7.13%) ⬇️
src/main/host/descriptor/descriptor_table.rs 80.48% <0.00%> (-2.44%) ⬇️
... and 14 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

let mut num_bytes_received = 0;
let mut num_bytes_received_since_eagain = 0;

// The socket's receive buffer should be full and we should be able to call recv() repeatedly to
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe worth having a second test here where the receiver takes too long to process packets, s.t. packets do get dropped? Maybe a nanosleep after processing each packet...? I'd expect such a test to pass now, but we'd want to be careful not to make it start failing when fixing #1341.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Build Build/install tools and dependencies Component: Testing Unit and integration tests and frameworks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Packets are dropped when net buffer is full even if receiver is trying to read
2 participants