From 664f355ee9c631afb4d80440e837e7b48028bbc8 Mon Sep 17 00:00:00 2001 From: Anand Inguva <34158215+AnandInguva@users.noreply.github.com> Date: Tue, 27 Jun 2023 15:04:05 -0400 Subject: [PATCH] Fix perf tool tests (#27270) * 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 --- .github/workflows/run_perf_alert_tool.yml | 6 ++++++ .../apache_beam/testing/analyzers/perf_analysis.py | 6 ++++-- .../testing/analyzers/perf_analysis_test.py | 14 ++++++++------ 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/.github/workflows/run_perf_alert_tool.yml b/.github/workflows/run_perf_alert_tool.yml index 510db83f8690..1cdcd858e61a 100644 --- a/.github/workflows/run_perf_alert_tool.yml +++ b/.github/workflows/run_perf_alert_tool.yml @@ -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. @@ -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 }} @@ -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. diff --git a/sdks/python/apache_beam/testing/analyzers/perf_analysis.py b/sdks/python/apache_beam/testing/analyzers/perf_analysis.py index e5e8a3a7b2fe..7f1ffbb944e9 100644 --- a/sdks/python/apache_beam/testing/analyzers/perf_analysis.py +++ b/sdks/python/apache_beam/testing/analyzers/perf_analysis.py @@ -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, @@ -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__': diff --git a/sdks/python/apache_beam/testing/analyzers/perf_analysis_test.py b/sdks/python/apache_beam/testing/analyzers/perf_analysis_test.py index 000175e6388a..fabf185a41d8 100644 --- a/sdks/python/apache_beam/testing/analyzers/perf_analysis_test.py +++ b/sdks/python/apache_beam/testing/analyzers/perf_analysis_test.py @@ -21,6 +21,7 @@ import unittest import mock +import numpy as np import pandas as pd # pylint: disable=ungrouped-imports @@ -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]) }]) @@ -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', @@ -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', @@ -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) @@ -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) @@ -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)