Skip to content

Commit

Permalink
Fix perf tool tests (#27270)
Browse files Browse the repository at this point in the history
* Fix tests

* Don't publish data when running analyzer in PR

* Update .github/workflows/run_perf_alert_tool.yml

* Run perf analysis tests during schedules

* Don't authenticate during pull_request
  • Loading branch information
AnandInguva authored Jun 27, 2023
1 parent 2f555c5 commit 664f355
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 8 deletions.
6 changes: 6 additions & 0 deletions .github/workflows/run_perf_alert_tool.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ on:
workflow_dispatch:
schedule:
- cron: '5 22 * * *'
pull_request:
branches: ['master']
tags: 'v*'
paths: ['sdks/python/apache_beam/testing/**']
jobs:
python_run_change_point_analysis:
name: Run Change Point Analysis.
Expand All @@ -37,6 +41,7 @@ jobs:
with:
python-version: 3.8
- name: Authenticate on GCP
if: github.event_name != 'pull_request'
uses: google-github-actions/setup-gcloud@v0
with:
service_account_key: ${{ secrets.GCP_SA_KEY }}
Expand All @@ -55,6 +60,7 @@ jobs:
working-directory: ./sdks/python/apache_beam/testing/analyzers
shell: bash
run: python perf_analysis.py
if: github.event_name != 'pull_request'
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
- name: Run change point analysis tests.
Expand Down
6 changes: 4 additions & 2 deletions sdks/python/apache_beam/testing/analyzers/perf_analysis.py
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,6 @@ def run_change_point_analysis(params, test_name, big_query_metrics_fetcher):
change_point_index=change_point_index,
timestamps=timestamps,
min_runs_between_change_points=min_runs_between_change_points)
logging.debug("Performance alert is %s for test %s" % (is_alert, test_name))
if is_alert:
issue_number, issue_url = create_performance_alert(
metric_name, test_name, timestamps,
Expand Down Expand Up @@ -173,7 +172,10 @@ def run(config_file_path: Optional[str] = None) -> None:
big_query_metrics_fetcher = BigQueryMetricsFetcher()

for test_name, params in tests_config.items():
run_change_point_analysis(params, test_name, big_query_metrics_fetcher)
run_change_point_analysis(
params=params,
test_name=test_name,
big_query_metrics_fetcher=big_query_metrics_fetcher)


if __name__ == '__main__':
Expand Down
14 changes: 8 additions & 6 deletions sdks/python/apache_beam/testing/analyzers/perf_analysis_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import unittest

import mock
import numpy as np
import pandas as pd

# pylint: disable=ungrouped-imports
Expand Down Expand Up @@ -54,7 +55,8 @@ def get_existing_issue_data(**kwargs):
# change point found at index 10. So passing 10 in the
# existing issue data in mock method.
return pd.DataFrame([{
constants._CHANGE_POINT_TIMESTAMP_LABEL: 10, constants._ISSUE_NUMBER: 0
constants._CHANGE_POINT_TIMESTAMP_LABEL: 10,
constants._ISSUE_NUMBER: np.array([0])
}])


Expand All @@ -70,7 +72,7 @@ def setUp(self) -> None:
] * 20
self.timestamps = list(range(5))
self.params = {
'test_name': 'fake_test',
'test_description': 'fake_description',
'metrics_dataset': 'fake_dataset',
'metrics_table': 'fake_table',
'project': 'fake_project',
Expand Down Expand Up @@ -102,7 +104,7 @@ def test_change_point_outside_inspection_window_is_not_a_valid_alert(self):

def test_validate_config(self):
test_keys = {
'test_name',
'test_description',
'metrics_dataset',
'metrics_table',
'project',
Expand Down Expand Up @@ -146,7 +148,7 @@ def test_duplicate_change_points_are_not_valid_alerts(self):
def test_no_alerts_when_no_change_points(self):
is_alert = analysis.run_change_point_analysis(
params=self.params,
test_id=self.test_id,
test_name=self.test_id,
big_query_metrics_fetcher=None)
self.assertFalse(is_alert)

Expand All @@ -167,7 +169,7 @@ def test_no_alerts_when_no_change_points(self):
def test_alert_on_data_with_change_point(self, *args):
is_alert = analysis.run_change_point_analysis(
params=self.params,
test_id=self.test_id,
test_name=self.test_id,
big_query_metrics_fetcher=None)
self.assertTrue(is_alert)

Expand All @@ -187,7 +189,7 @@ def test_alert_on_data_with_change_point(self, *args):
def test_alert_on_data_with_reported_change_point(self, *args):
is_alert = analysis.run_change_point_analysis(
params=self.params,
test_id=self.test_id,
test_name=self.test_id,
big_query_metrics_fetcher=None)
self.assertFalse(is_alert)

Expand Down

0 comments on commit 664f355

Please sign in to comment.