Fix test_resolve_gitapi_subapps benchmark

[asvetlov]

Also add checks that url dispatcher benchmarks find expected routes

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.72%. Comparing base (69ca7df) to head (0e3d123).
Report is 1 commits behind head on master.

✅ All tests successful. No failed tests found.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #9939      +/-   ##
==========================================
+ Coverage   98.70%   98.72%   +0.02%     
==========================================
  Files         121      121              
  Lines       36621    36682      +61     
  Branches     4376     4378       +2     
==========================================
+ Hits        36146    36214      +68     
+ Misses        319      315       -4     
+ Partials      156      153       -3     

Flag	Coverage Δ
CI-GHA	98.61% <100.00%> (+0.01%)	⬆️
OS-Linux	98.30% <100.00%> (+0.10%)	⬆️
OS-Windows	96.12% <100.00%> (+0.10%)	⬆️
OS-macOS	97.39% <100.00%> (+<0.01%)	⬆️
Py-3.10.11	97.24% <100.00%> (+<0.01%)	⬆️
Py-3.10.15	97.83% <100.00%> (+0.04%)	⬆️
Py-3.11.10	97.84% <100.00%> (+<0.01%)	⬆️
Py-3.11.9	97.30% <100.00%> (+0.31%)	⬆️
Py-3.12.7	98.36% <100.00%> (+0.04%)	⬆️
Py-3.13.0	98.30% <100.00%> (+0.94%)	⬆️
Py-3.9.13	97.16% <100.00%> (+0.30%)	⬆️
Py-3.9.20	97.75% <100.00%> (?)
Py-pypy7.3.16	97.32% <100.00%> (?)
VM-macos	97.39% <100.00%> (+<0.01%)	⬆️
VM-ubuntu	98.30% <100.00%> (+0.10%)	⬆️
VM-windows	96.12% <100.00%> (+0.10%)	⬆️

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

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

[codspeed-hq]

CodSpeed Performance Report

Merging #9939 will degrade performances by 97.31%

_{Comparing fix-subapps-gitapi-benchmark (0e3d123) with master (69ca7df)}

Summary

❌ 1 (👁 1) regressions
✅ 41 untouched benchmarks

Benchmarks breakdown

	Benchmark	master	fix-subapps-gitapi-benchmark	Change
👁	test_resolve_gitapi_subapps[pyloop]	8.6 ms	319.7 ms	-97.31%

[asvetlov]

I think the speed decrease is acceptable.
Checks add a constant execution time, it's ok.
Better to have checked results; it prevents silly errors like I made for test_resolve_gitapi_subapps

[bdraco]

I'd probably write it like this to avoid
the assert inside the benchmark

diff --git a/tests/test_benchmarks_web_urldispatcher.py b/tests/test_benchmarks_web_urldispatcher.py
index bb6cf3aa3..ef1f2c4e8 100644
--- a/tests/test_benchmarks_web_urldispatcher.py
+++ b/tests/test_benchmarks_web_urldispatcher.py
@@ -53,11 +53,17 @@ def test_resolve_root_route(
     router = app.router
     request = _mock_request(method="GET", path="/")
 
-    async def run_url_dispatcher_benchmark() -> None:
+    async def run_url_dispatcher_assertions() -> None:
         for _ in range(resolve_count):
             ret = await router.resolve(request)
             assert ret.get_info()["path"] == "/", ret.get_info()
 
+    loop.run_until_complete(run_url_dispatcher_assertions())
+
+    async def run_url_dispatcher_benchmark() -> None:
+        for _ in range(resolve_count):
+            await router.resolve(request)
+
     @benchmark
     def _run() -> None:
         loop.run_until_complete(run_url_dispatcher_benchmark())

Sorry, no inline suggestion as I'm in flight and WiFi is like a 9600 baud modem

[Dreamsorcerer]

Checks add a constant execution time, it's ok.

Hopefully bdraco's suggestion resolves the issue. Otherwise, the drop in test_resolve_gitapi_subapps[pyloop] looks too big. If 97% of the time is spent on the new asserts, we'd be unable to detect any improvements to the benchmark.

[asvetlov]

No, for GitHub subapps I'fixed the benchmark.
Previously it returned 404 Not Found instead of 200 Ok, fail path was faster than lookup into the subapp

[asvetlov]

@bdraco I've got your idea.
I'll rewrite benchmarks to perform assertions outside of measured code.
Thanks for the idea!

[asvetlov]

Now the only test_resolve_gitapi_subapps is about two times slower because I've fixed this incorrect benchmark.

[asvetlov]

@bdraco thanks for the fast review.
Do you sleep sometimes? :)

[bdraco]

Not so much at the end of the year as its always travel heavy for me and I'm flipping time zones every few days.

[patchback]

Backport to 3.10: 💚 backport PR created

✅ Backport PR branch: patchback/backports/3.10/509fddfd3c6673bfa5ad11783e8d49e57d5a2fbe/pr-9939

Backported as #9948

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

[patchback]

Backport to 3.11: 💚 backport PR created

✅ Backport PR branch: patchback/backports/3.11/509fddfd3c6673bfa5ad11783e8d49e57d5a2fbe/pr-9939

Backported as #9949

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

[patchback]

Backport to 3.12: 💚 backport PR created

✅ Backport PR branch: patchback/backports/3.12/509fddfd3c6673bfa5ad11783e8d49e57d5a2fbe/pr-9939

Backported as #9950

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.