-
Notifications
You must be signed in to change notification settings - Fork 127
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
fix(transport): remove unused Error::HandshakeFailed #2160
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2160 +/- ##
=======================================
Coverage 95.39% 95.39%
=======================================
Files 112 112
Lines 36373 36373
=======================================
Hits 34697 34697
Misses 1676 1676 ☔ View full report in Codecov by Sentry. |
Was that ever used? Or was the code that used it maybe inadvertently removed, like for |
Instantiation of |
Failed Interop TestsQUIC Interop Runner, client vs. server neqo-latest as client
neqo-latest as server
All resultsSucceeded Interop TestsQUIC Interop Runner, client vs. server neqo-latest as client
neqo-latest as server
Unsupported Interop TestsQUIC Interop Runner, client vs. server neqo-latest as client
neqo-latest as server
|
Benchmark resultsPerformance differences relative to e11528d. coalesce_acked_from_zero 1+1 entries: No change in performance detected.time: [99.254 ns 99.559 ns 99.869 ns] change: [-0.8069% -0.2405% +0.3239%] (p = 0.43 > 0.05) coalesce_acked_from_zero 3+1 entries: No change in performance detected.time: [117.63 ns 118.00 ns 118.38 ns] change: [-0.2552% +0.1396% +0.5368%] (p = 0.48 > 0.05) coalesce_acked_from_zero 10+1 entries: No change in performance detected.time: [117.08 ns 117.46 ns 117.95 ns] change: [-0.3217% +0.0871% +0.5269%] (p = 0.69 > 0.05) coalesce_acked_from_zero 1000+1 entries: No change in performance detected.time: [98.339 ns 104.11 ns 116.66 ns] change: [-0.4832% +2.0889% +6.5111%] (p = 0.37 > 0.05) RxStreamOrderer::inbound_frame(): Change within noise threshold.time: [111.02 ms 111.07 ms 111.12 ms] change: [-0.4327% -0.3572% -0.2849%] (p = 0.00 < 0.05) transfer/pacing-false/varying-seeds: No change in performance detected.time: [26.947 ms 27.878 ms 28.822 ms] change: [-2.3815% +2.3310% +7.2711%] (p = 0.36 > 0.05) transfer/pacing-true/varying-seeds: No change in performance detected.time: [35.354 ms 36.950 ms 38.553 ms] change: [-8.0779% -1.1205% +6.0925%] (p = 0.75 > 0.05) transfer/pacing-false/same-seed: No change in performance detected.time: [25.550 ms 26.425 ms 27.299 ms] change: [-6.5155% -2.4019% +1.9146%] (p = 0.30 > 0.05) transfer/pacing-true/same-seed: No change in performance detected.time: [41.178 ms 43.185 ms 45.165 ms] change: [-5.5296% +1.0699% +7.7072%] (p = 0.74 > 0.05) 1-conn/1-100mb-resp (aka. Download)/client: 💚 Performance has improved.time: [112.68 ms 113.14 ms 113.57 ms] thrpt: [880.50 MiB/s 883.86 MiB/s 887.47 MiB/s] change: time: [-2.3237% -1.7248% -1.1158%] (p = 0.00 < 0.05) thrpt: [+1.1284% +1.7550% +2.3790%] 1-conn/10_000-parallel-1b-resp (aka. RPS)/client: Change within noise threshold.time: [309.09 ms 312.84 ms 316.56 ms] thrpt: [31.590 Kelem/s 31.966 Kelem/s 32.354 Kelem/s] change: time: [-3.5281% -1.8367% -0.0873%] (p = 0.04 < 0.05) thrpt: [+0.0874% +1.8710% +3.6571%] 1-conn/1-1b-resp (aka. HPS)/client: No change in performance detected.time: [33.926 ms 34.105 ms 34.310 ms] thrpt: [29.146 elem/s 29.321 elem/s 29.476 elem/s] change: time: [-0.3867% +0.4658% +1.3382%] (p = 0.29 > 0.05) thrpt: [-1.3205% -0.4637% +0.3882%] Client/server transfer resultsTransfer of 33554432 bytes over loopback.
|
@mxinden it seems to be used in the glue: https://github.com/mozilla/neqo/actions/runs/11251536635/job/31282794009?pr=2163#step:9:1163 |
Yes, TransportError::HandshakeFailed => CloseError::TransportInternalErrorOther(6), |
Ok, so we just need to remove it there. Really wish we could compile the glue together with neqo somehow in CI (without the full Fx). |
I guess we could run neqo/.github/workflows/firefox.yml Line 122 in f39e12e
That would still suffice as a smoke test, but won't give us a full Firefox build. |
I was hoping we could just do a sparse checkout and build the |
No description provided.