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

test(transport/vn): allow dropped_rx of 1 #2363

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

Conversation

mxinden
Copy link
Collaborator

@mxinden mxinden commented Jan 16, 2025

compatible_upgrade_large_initial tests whether Neqo can handle an ACK using QUIC version Y on a connection that ends up using QUIC version X.

#2228 changed the test, asserting that the server will drop 2 instead of 1 packet. This spuriosly fails on CI:

https://github.com/mozilla/neqo/actions/runs/12796212878/job/35675377711?pr=2295

This commit allows both 1 and 2 packets dropped by the server.


@larseggert based on your changes in #2228, is this a valid assumption to make?

`compatible_upgrade_large_initial` tests whether Neqo can handle an ACK using
QUIC version Y on a connection that ends up using QUIC version X.

mozilla#2228 changed the test, asserting that the
server will drop 2 instead of 1 packet. This spuriosly fails on CI:

https://github.com/mozilla/neqo/actions/runs/12796212878/job/35675377711?pr=2295

This commit allows both 1 and 2 packets dropped by the server.
Copy link

Failed Interop Tests

QUIC Interop Runner, client vs. server, differences relative to 7f8136e.

neqo-latest as client

neqo-latest as server

All results

Succeeded Interop Tests

QUIC Interop Runner, client vs. server

neqo-latest as client

neqo-latest as server

Unsupported Interop Tests

QUIC Interop Runner, client vs. server

neqo-latest as client

neqo-latest as server

Copy link

codecov bot commented Jan 16, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.29%. Comparing base (6b87603) to head (f289869).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2363      +/-   ##
==========================================
- Coverage   95.29%   95.29%   -0.01%     
==========================================
  Files         114      114              
  Lines       36856    36843      -13     
  Branches    36856    36843      -13     
==========================================
- Hits        35123    35110      -13     
  Misses       1727     1727              
  Partials        6        6              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@larseggert
Copy link
Collaborator

I can't recall why I made this change. So with #2228, is it that sometimes we have one and sometimes we have two packets we drop?

(As part of #2360, there will be a connection parameter to turn off SNI slicing. We can enable that here and revert the condition to the one from before #2228.)

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