Skip to content

Measure the performance in launchable record tests command #966

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.

Sign up for GitHub

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

Merged
ono-max merged 1 commit into from
Jan 22, 2025
Merged

Measure the performance in launchable record tests command #966

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions launchable/commands/helper.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from time import time
from typing import Optional, Sequence, Tuple

import click
Expand Down Expand Up @@ -137,6 +138,12 @@ def find_or_create_session(
return read_session(saved_build_name)


def time_ns():
# time.time_ns() method is new in Python version 3.7
# As a workaround, we convert time.time() to nanoseconds.
return int(time() * 1e9)
Comment on lines +141 to +144
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add type hints and documentation for the time_ns() function.

The function should follow the codebase's conventions for type hints and documentation.

Apply this diff to improve the function:

-def time_ns():
+def time_ns() -> int:
+    """Return the current time in nanoseconds.
+    
+    This is a compatibility function for Python versions < 3.7 that don't have time.time_ns().
+    Note: The precision might be lower than actual time.time_ns() due to floating-point arithmetic.
+    
+    Returns:
+        int: Current time in nanoseconds
+    """
     # time.time_ns() method is new in Python version 3.7
     # As a workaround, we convert time.time() to nanoseconds.
     return int(time() * 1e9)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def time_ns():
# time.time_ns() method is new in Python version 3.7
# As a workaround, we convert time.time() to nanoseconds.
return int(time() * 1e9)
def time_ns() -> int:
"""Return the current time in nanoseconds.
This is a compatibility function for Python versions < 3.7 that don't have time.time_ns().
Note: The precision might be lower than actual time.time_ns() due to floating-point arithmetic.
Returns:
int: Current time in nanoseconds
"""
# time.time_ns() method is new in Python version 3.7
# As a workaround, we convert time.time() to nanoseconds.
return int(time() * 1e9)



def _check_observation_mode_status(session: str, is_observation: bool,
tracking_client: TrackingClient, app: Optional[Application] = None):
if not is_observation:
Expand Down
20 changes: 19 additions & 1 deletion launchable/commands/record/tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
from ...utils.logger import Logger
from ...utils.no_build import NO_BUILD_BUILD_NAME, NO_BUILD_TEST_SESSION_ID
from ...utils.session import parse_session, read_build
from ..helper import find_or_create_session
from ..helper import find_or_create_session, time_ns
from .case_event import CaseEvent, CaseEventType

GROUP_NAME_RULE = re.compile("^[a-zA-Z0-9][a-zA-Z0-9_-]*$")
Expand Down Expand Up @@ -535,14 +535,24 @@ def recorded_result() -> Tuple[int, int, int, float]:
return test_count, success_count, fail_count, duration / 60 # sec to min

try:
start = time_ns()
tc = testcases(self.reports)
end = time_ns()
tracking_client.send_event(
event_name=Tracking.Event.PERFORMANCE,
metadata={
"elapsedTime": end - start,
"measurementTarget": "testcases method(parsing report file)"
}
)

if report_paths:
# diagnostics mode to just report test paths
for t in tc:
print(unparse_test_path(t['testPath']))
return

start = time_ns()
exceptions = []
for chunk in ichunked(tc, post_chunk):
p, es = payload(
Expand All @@ -555,6 +565,14 @@ def recorded_result() -> Tuple[int, int, int, float]:

send(p)
exceptions.extend(es)
end = time_ns()
tracking_client.send_event(
event_name=Tracking.Event.PERFORMANCE,
metadata={
"elapsedTime": end - start,
"measurementTarget": "events API"
}
)

if len(exceptions) > 0:
raise Exception(exceptions)
Expand Down
1 change: 1 addition & 0 deletions launchable/utils/tracking.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ class Tracking:
# General events
class Event(Enum):
SHALLOW_CLONE = 'SHALLOW_CLONE' # this event is an example
PERFORMANCE = 'PERFORMANCE'

# Error events
class ErrorEvent(Enum):
Expand Down
2 changes: 1 addition & 1 deletion tests/commands/test_api_error.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,7 @@ def test_all_workflow_when_server_down(self):
result = self.cli("record", "tests", "--session", self.session, "minitest", str(self.test_files_dir) + "/")
self.assert_success(result)
# Since Timeout error is caught inside of LaunchableClient, the tracking event is sent twice.
self.assert_tracking_count(tracking=tracking, count=8)
self.assert_tracking_count(tracking=tracking, count=9)

def assert_tracking_count(self, tracking, count: int):
# Prior to 3.6, `Response` object can't be obtained.
Expand Down