chore: rely on nss-rs for NSS initialization and test DB#3621
chore: rely on nss-rs for NSS initialization and test DB#3621larseggert wants to merge 2 commits into
Conversation
|
Depends on mozilla/nss-rs#56 |
4824dad to
9f9ba48
Compare
martinthomson
left a comment
There was a problem hiding this comment.
I can't see a problem here, other than the challenge of finding the db from the NSS crate (which I hope the tests will sort out)
9f9ba48 to
f885bcc
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3621 +/- ##
==========================================
- Coverage 95.11% 95.01% -0.11%
==========================================
Files 111 116 +5
Lines 37999 38357 +358
Branches 37999 38357 +358
==========================================
+ Hits 36144 36443 +299
- Misses 1159 1215 +56
- Partials 696 699 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
|
0ca0d67 to
aaa87c0
Compare
Delegate NSS init in `fixture_init` to `nss-rs`'s own `test-fixture` crate instead of duplicating the logic. Remove `test-fixture/db` — the NSS database now comes from `nss-rs`, which exposes `TEST_FIXTURE_DB` as the default path for `neqo-server --db`. Update CI and scripts accordingly.
aaa87c0 to
f9af700
Compare
Client/server transfer resultsPerformance differences relative to 7b0962d. No significant performance differences. Table above only shows statistically significant changes. See all results below. All resultsTransfer of 33554432 bytes over loopback, min. 100 runs. All unit-less numbers are in milliseconds.
Download data for |
There was a problem hiding this comment.
Pull request overview
This PR moves Neqo’s NSS test database setup to the nss-rs test fixture, removing the in-repo test DB dependency and updating server defaults, tests, docs, and CI packaging to use the nss-rs-provided database.
Changes:
- Delegates
test-fixtureNSS initialization tonss-test-fixture. - Makes
neqo-server --dboptional and defaults it to the nss-rs fixture DB. - Updates docs, benchmarks, scripts, and workflows to stop depending on
test-fixture/db.
Reviewed changes
Copilot reviewed 18 out of 21 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
test/upload_test.sh |
Removes explicit server DB path. |
test/README.md |
Updates upload test instructions for default DB behavior. |
test-fixture/src/lib.rs |
Delegates NSS fixture setup to nss-rs. |
test-fixture/db/pkcs11.txt |
Removes committed NSS DB metadata. |
test-fixture/Cargo.toml |
Adds nss-rs test fixture dependency. |
README.md |
Updates local Firefox testing command. |
neqo-bin/src/server/mod.rs |
Makes server DB argument optional and uses nss-rs fixture default. |
neqo-bin/src/lib.rs |
Uses shared test fixture initialization in tests. |
neqo-bin/Cargo.toml |
Adds nss fixture dependencies for server/tests/benches. |
neqo-bin/benches/main.rs |
Uses shared fixture initialization in benchmarks. |
Cargo.toml |
Updates nss-rs and adds nss test fixture workspace dependency. |
Cargo.lock |
Locks nss-rs 0.11.0 and the new fixture package. |
AGENTS.md |
Updates contributor guidance for NSS setup. |
.github/workflows/profile.yml |
Removes old NSS DB env usage from profiling jobs. |
.github/workflows/perfcompare.yml |
Exports the new fixture DB environment for perf comparison. |
.github/workflows/bench.yml |
Exports and forwards the new fixture DB environment for benchmarks. |
.github/scripts/perfcompare.py |
Preserves the new fixture DB env through sudo. |
.github/actions/check-android/action.yml |
Pushes the nss-rs fixture DB for Android tests. |
.github/actions/build-neqo/action.yml |
Packages the nss-rs fixture DB into build artifacts. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Lars Eggert <lars@eggert.org>
Benchmark resultsSignificant performance differences relative to 7b0962d. streams-flow-controlled/walltime/1-streams/each-4194304-bytes: 💔 Performance has regressed by +1.3004%. time: [33.063 ms 33.143 ms 33.252 ms]
change: [+1.0248% +1.3004% +1.6832] (p = 0.00 < 0.05)
Performance has regressed.
Found 2 outliers among 100 measurements (2.00%)
1 (1.00%) high mild
1 (1.00%) high severeAll resultstransfer/1-conn/1-100mb-resp (aka. Download): No change in performance detected. time: [184.00 ms 184.55 ms 185.18 ms]
thrpt: [540.01 MiB/s 541.87 MiB/s 543.48 MiB/s]
change:
time: [-0.1493% +0.2150% +0.5853] (p = 0.27 > 0.05)
thrpt: [-0.5819% -0.2145% +0.1495]
No change in performance detected.
Found 3 outliers among 100 measurements (3.00%)
2 (2.00%) high mild
1 (1.00%) high severetransfer/1-conn/10_000-parallel-1b-resp (aka. RPS): No change in performance detected. time: [279.10 ms 281.19 ms 283.27 ms]
thrpt: [35.302 Kelem/s 35.563 Kelem/s 35.830 Kelem/s]
change:
time: [-1.7070% -0.7410% +0.2904] (p = 0.16 > 0.05)
thrpt: [-0.2896% +0.7466% +1.7367]
No change in performance detected.transfer/1-conn/1-1b-resp (aka. HPS): No change in performance detected. time: [39.103 ms 39.261 ms 39.438 ms]
thrpt: [25.356 B/s 25.470 B/s 25.573 B/s]
change:
time: [-0.6635% -0.0463% +0.5660] (p = 0.89 > 0.05)
thrpt: [-0.5628% +0.0463% +0.6679]
No change in performance detected.
Found 12 outliers among 100 measurements (12.00%)
6 (6.00%) high mild
6 (6.00%) high severetransfer/1-conn/1-100mb-req (aka. Upload): Change within noise threshold. time: [186.25 ms 186.55 ms 186.85 ms]
thrpt: [535.19 MiB/s 536.05 MiB/s 536.90 MiB/s]
change:
time: [-1.0115% -0.7697% -0.5460] (p = 0.00 < 0.05)
thrpt: [+0.5490% +0.7757% +1.0218]
Change within noise threshold.
Found 2 outliers among 100 measurements (2.00%)
2 (2.00%) high mildstreams/walltime/1-streams/each-1000-bytes: Change within noise threshold. time: [587.57 µs 589.73 µs 592.23 µs]
change: [-1.4532% -0.8689% -0.2521] (p = 0.00 < 0.05)
Change within noise threshold.
Found 11 outliers among 100 measurements (11.00%)
1 (1.00%) high mild
10 (10.00%) high severestreams/walltime/1000-streams/each-1-bytes: Change within noise threshold. time: [11.995 ms 12.018 ms 12.041 ms]
change: [-0.9736% -0.7213% -0.4686] (p = 0.00 < 0.05)
Change within noise threshold.
Found 1 outliers among 100 measurements (1.00%)
1 (1.00%) high mildstreams/walltime/1000-streams/each-1000-bytes: Change within noise threshold. time: [42.558 ms 42.598 ms 42.638 ms]
change: [+0.4327% +0.6444% +0.8216] (p = 0.00 < 0.05)
Change within noise threshold.
Found 1 outliers among 100 measurements (1.00%)
1 (1.00%) high mildstreams-flow-controlled/walltime/1-streams/each-4194304-bytes: 💔 Performance has regressed by +1.3004%. time: [33.063 ms 33.143 ms 33.252 ms]
change: [+1.0248% +1.3004% +1.6832] (p = 0.00 < 0.05)
Performance has regressed.
Found 2 outliers among 100 measurements (2.00%)
1 (1.00%) high mild
1 (1.00%) high severestreams-flow-controlled/walltime/10-streams/each-1048576-bytes: No change in performance detected. time: [94.317 ms 95.564 ms 96.852 ms]
change: [-2.1300% -0.2900% +1.7933] (p = 0.77 > 0.05)
No change in performance detected.transfer/walltime/pacing-false/varying-seeds: Change within noise threshold. time: [22.417 ms 22.430 ms 22.444 ms]
change: [-0.6687% -0.5135% -0.3773] (p = 0.00 < 0.05)
Change within noise threshold.
Found 2 outliers among 100 measurements (2.00%)
2 (2.00%) high mildtransfer/walltime/pacing-true/varying-seeds: Change within noise threshold. time: [22.744 ms 22.759 ms 22.775 ms]
change: [-0.6230% -0.5049% -0.3930] (p = 0.00 < 0.05)
Change within noise threshold.
Found 1 outliers among 100 measurements (1.00%)
1 (1.00%) high mildtransfer/walltime/pacing-false/same-seed: Change within noise threshold. time: [22.340 ms 22.355 ms 22.372 ms]
change: [-0.8931% -0.7760% -0.6577] (p = 0.00 < 0.05)
Change within noise threshold.
Found 2 outliers among 100 measurements (2.00%)
2 (2.00%) high mildtransfer/walltime/pacing-true/same-seed: Change within noise threshold. time: [22.561 ms 22.575 ms 22.588 ms]
change: [-1.4927% -1.3321% -1.1953] (p = 0.00 < 0.05)
Change within noise threshold.
Found 1 outliers among 100 measurements (1.00%)
1 (1.00%) high mildDownload data for |
Failed Interop TestsQUIC Interop Runner, client vs. server, differences relative to
All resultsSucceeded Interop TestsQUIC Interop Runner, client vs. server neqo-pr as client
neqo-pr as server
Unsupported Interop TestsQUIC Interop Runner, client vs. server neqo-pr as client
neqo-pr as server
|
Delegate NSS init in
fixture_inittonss-rs's owntest-fixturecrate instead of duplicating the logic. Removetest-fixture/db— the NSS database now comes fromnss-rs, which exposesTEST_FIXTURE_DBas the default path forneqo-server --db. Update CI and scripts accordingly.