Sync changes from internal dev branch.#67
Conversation
PiperOrigin-RevId: 770194711
PiperOrigin-RevId: 775486936
PiperOrigin-RevId: 785404369
Summary of ChangesHello @lip-liu, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request integrates substantial updates from an internal development branch, primarily focusing on a significant overhaul of the project's underlying build and packaging infrastructure. Concurrently, it introduces new testing capabilities to assess Nearby Connections performance under specific network conditions, enhancing the suite's coverage for complex multi-device interactions. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
PiperOrigin-RevId: 787233899
PiperOrigin-RevId: 788179678
PiperOrigin-RevId: 788213054
PiperOrigin-RevId: 788669201
PiperOrigin-RevId: 789088205
PiperOrigin-RevId: 795668759
PiperOrigin-RevId: 797384842
PiperOrigin-RevId: 799316329
PiperOrigin-RevId: 800267639
There was a problem hiding this comment.
Code Review
This pull request syncs changes from an internal development branch, which includes removing several build and configuration files, and adding a new performance test for Nearby Connections with multiple advertisers. My review focuses on the new test file. I've identified a high-severity issue in the test's teardown logic that could lead to a crash and mask other failures, an unnecessary operation that consumes device resources, and a point of poor encapsulation that affects maintainability. I've provided suggestions to fix the crash and improve the code.
I am having trouble creating individual review comments. Click here to see my feedback.
betocq/performance_tests/nc_multi_advertisers_test.py (216-228)
There's a potential for a ZeroDivisionError if self._test_count is 0, which can happen if all test iterations fail before the counter is incremented. Similarly, max(), min(), and statistics.median() will raise exceptions if self._connection_time_list is empty.
This can cause teardown_class to crash, which might hide the root cause of the actual test failures. It's safer to check if there are any results before trying to calculate and record statistics.
if self._test_count > 0:
self.record_data({
'Test Class': self.TAG,
'properties': {
'avg_connection_time (sec)': (
self._sum_connection_time_sec / self._test_count
),
'max_connection_time (sec)': max(self._connection_time_list),
'median_connection_time (sec)': statistics.median(
self._connection_time_list
),
'min_connection_time (sec)': min(self._connection_time_list),
},
})
betocq/performance_tests/nc_multi_advertisers_test.py (94-98)
Discovery is being started on all advertiser devices, but the returned callback handlers are never used. This appears to be unnecessary and consumes device resources (CPU, battery, and radio time). If this is intended to create a more realistic, noisy environment, it should be documented with a comment. Otherwise, this block of code should be removed.
betocq/performance_tests/nc_multi_advertisers_test.py (135-138)
Accessing private members (_advertiser_connection_lifecycle_callback and _advertiser_endpoint_id) of the nearby_connection_wrapper object violates encapsulation principles. This makes the code brittle, as future changes to the NearbyConnectionWrapper class could break this test.
A better approach would be to refactor NearbyConnectionWrapper to expose a public API that supports this use case (where discovery is performed externally). For now, this can be considered technical debt, but it should be addressed to improve maintainability.
PiperOrigin-RevId: 800735347
PiperOrigin-RevId: 802848866
PiperOrigin-RevId: 802985717
PiperOrigin-RevId: 803563740
PiperOrigin-RevId: 808719785
PiperOrigin-RevId: 813786655
PiperOrigin-RevId: 813834298
PiperOrigin-RevId: 813904215
PiperOrigin-RevId: 814495136
PiperOrigin-RevId: 816780456
PiperOrigin-RevId: 820846798
PiperOrigin-RevId: 820883137
PiperOrigin-RevId: 821720888
PiperOrigin-RevId: 821768960
PiperOrigin-RevId: 822116903
PiperOrigin-RevId: 822119710
PiperOrigin-RevId: 837114151
PiperOrigin-RevId: 837211845
PiperOrigin-RevId: 837212774
PiperOrigin-RevId: 838889392
PiperOrigin-RevId: 838916830
PiperOrigin-RevId: 839354317
PiperOrigin-RevId: 839958549
PiperOrigin-RevId: 841858968
PiperOrigin-RevId: 841866448
PiperOrigin-RevId: 842347592
PiperOrigin-RevId: 842348630
PiperOrigin-RevId: 842350854
PiperOrigin-RevId: 842369164
PiperOrigin-RevId: 842776389
PiperOrigin-RevId: 842870609
PiperOrigin-RevId: 843398332
PiperOrigin-RevId: 843814754
PiperOrigin-RevId: 843889477
PiperOrigin-RevId: 843909588
PiperOrigin-RevId: 844867868
PiperOrigin-RevId: 844874616
PiperOrigin-RevId: 846021507
PiperOrigin-RevId: 846281667
PiperOrigin-RevId: 846400441
PiperOrigin-RevId: 846512677
PiperOrigin-RevId: 853771884
PiperOrigin-RevId: 853834511
No description provided.