-
Notifications
You must be signed in to change notification settings - Fork 176
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: 3eaa4c9 | Previous: 3feffc9 | Ratio |
|---|---|---|---|
rule-match-browserlike/brave-list |
2030109024 ns/iter (± 15093842) |
2089868473 ns/iter (± 20161585) |
0.97 |
rule-match-first-request/brave-list |
1103651 ns/iter (± 7955) |
1119548 ns/iter (± 13496) |
0.99 |
blocker_new/brave-list |
147048357 ns/iter (± 919282) |
155371876 ns/iter (± 816509) |
0.95 |
blocker_new/brave-list-deserialize |
23893548 ns/iter (± 154252) |
29471071 ns/iter (± 1288134) |
0.81 |
memory-usage/brave-list-initial |
10213344 ns/iter (± 3) |
10213344 ns/iter (± 3) |
1 |
memory-usage/brave-list-initial/max |
60612235 ns/iter (± 3) |
60612235 ns/iter (± 3) |
1 |
memory-usage/brave-list-initial/alloc-count |
1231711 ns/iter (± 3) |
1231711 ns/iter (± 3) |
1 |
memory-usage/brave-list-1000-requests |
2692712 ns/iter (± 3) |
2692696 ns/iter (± 3) |
1.00 |
memory-usage/brave-list-1000-requests/alloc-count |
71607 ns/iter (± 3) |
71591 ns/iter (± 3) |
1.00 |
url_cosmetic_resources/brave-list |
191515 ns/iter (± 644) |
192025 ns/iter (± 3720) |
1.00 |
cosmetic-class-id-match/brave-list |
3373510 ns/iter (± 893048) |
3479706 ns/iter (± 936426) |
0.97 |
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.
3ed31a6 to
4ccac3b
Compare
4ccac3b to
3eaa4c9
Compare
Related brave/brave-browser#50644
The PR moves test_utils and drops some of
#[path=..]usage.