-
Notifications
You must be signed in to change notification settings - Fork 175
Change test structure 1 #550
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rust Benchmark
| Benchmark suite | Current: 5f03dd3 | Previous: fca4bc5 | Ratio |
|---|---|---|---|
rule-match-browserlike/brave-list |
2217016115 ns/iter (± 18757134) |
2217231164 ns/iter (± 9972499) |
1.00 |
rule-match-first-request/brave-list |
1097105 ns/iter (± 41392) |
1138984 ns/iter (± 12605) |
0.96 |
blocker_new/brave-list |
168163453 ns/iter (± 1222404) |
165009583 ns/iter (± 313867) |
1.02 |
blocker_new/brave-list-deserialize |
23468735 ns/iter (± 205307) |
23484976 ns/iter (± 262551) |
1.00 |
memory-usage/brave-list-initial |
10212240 ns/iter (± 3) |
10212224 ns/iter (± 3) |
1.00 |
memory-usage/brave-list-initial/max |
62256263 ns/iter (± 3) |
62256247 ns/iter (± 3) |
1.00 |
memory-usage/brave-list-initial/alloc-count |
1362341 ns/iter (± 3) |
1362325 ns/iter (± 3) |
1.00 |
memory-usage/brave-list-1000-requests |
2666974 ns/iter (± 3) |
2666958 ns/iter (± 3) |
1.00 |
memory-usage/brave-list-1000-requests/alloc-count |
71385 ns/iter (± 3) |
71369 ns/iter (± 3) |
1.00 |
url_cosmetic_resources/brave-list |
199017 ns/iter (± 2056) |
202616 ns/iter (± 728) |
0.98 |
cosmetic-class-id-match/brave-list |
3356599 ns/iter (± 928738) |
3413013 ns/iter (± 963790) |
0.98 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors test utilities from an ad-hoc #[path] import pattern to a proper module structure. The test helpers are moved from tests/test_utils.rs to src/test_utils/mod.rs and exposed as a public module, allowing tests and benchmarks to import them via standard module paths.
Key changes:
- Moved test utilities from
tests/test_utils.rstosrc/test_utils/mod.rsas a public module - Updated all tests and benchmarks to use
adblock::test_utilsinstead of local#[path]imports - Reorganized flatbuffer generated code into
src/flatbuffers/mod.rsfor better module structure
Reviewed Changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/lib.rs | Exposes test_utils as a public module instead of a test-only module |
| src/test_utils/mod.rs | Updated module documentation to reflect its use in tests and benchmarks |
| tests/unit/engine.rs | Refactored imports to use standard module path for test utilities |
| tests/ublock-coverage.rs | Replaced local module import with public module import |
| tests/matching.rs | Removed #[path] directive in favor of standard import |
| benches/*.rs | Updated all benchmark files to use the new import path |
| src/flatbuffers/mod.rs | Added generated flatbuffer module declaration |
| src/filters/mod.rs | Removed inline flatbuffer module declaration |
| src/filters/fb_network_builder.rs | Updated to use new flatbuffer module path |
| src/filters/fb_builder.rs | Updated to use new flatbuffer module path |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Related brave/brave-browser#50644
The PR moves test_utils and drops some of
#[path=..]usage.