From 2ccb473583d5430f427bec9a74791a4807eefc0b Mon Sep 17 00:00:00 2001 From: Takashi Matsuo Date: Thu, 16 Apr 2020 13:40:56 -0700 Subject: [PATCH] [dlp] fix: remove gcp-devrel-py-tools fixes #3375 fixes #3416 fixes #3417 * remove wrong usage of `eventually_consistent.call` * only test if the operation has been started * shorter timeout for polling * correct use of `pytest.mark.flaky` --- dlp/inspect_content.py | 3 + dlp/inspect_content_test.py | 129 +++++++++++++++++++++--------------- dlp/jobs_test.py | 3 +- dlp/requirements-test.txt | 1 - dlp/risk_test.py | 24 +++---- 5 files changed, 89 insertions(+), 71 deletions(-) diff --git a/dlp/inspect_content.py b/dlp/inspect_content.py index b1e97ae31475..9e50ecbdf79d 100644 --- a/dlp/inspect_content.py +++ b/dlp/inspect_content.py @@ -474,6 +474,7 @@ def inspect_gcs_file( operation = dlp.create_dlp_job(parent, inspect_job=inspect_job) print("Inspection operation started: {}".format(operation.name)) + # Create a Pub/Sub client and find the subscription. The subscription is # expected to already be listening to the topic. subscriber = google.cloud.pubsub.SubscriberClient() @@ -636,6 +637,7 @@ def inspect_datastore( } operation = dlp.create_dlp_job(parent, inspect_job=inspect_job) + print("Inspection operation started: {}".format(operation.name)) # Create a Pub/Sub client and find the subscription. The subscription is # expected to already be listening to the topic. @@ -802,6 +804,7 @@ def inspect_bigquery( } operation = dlp.create_dlp_job(parent, inspect_job=inspect_job) + print("Inspection operation started: {}".format(operation.name)) # Create a Pub/Sub client and find the subscription. The subscription is # expected to already be listening to the topic. diff --git a/dlp/inspect_content_test.py b/dlp/inspect_content_test.py index ad493ecce710..be2b821044ee 100644 --- a/dlp/inspect_content_test.py +++ b/dlp/inspect_content_test.py @@ -15,8 +15,6 @@ import os import uuid -from gcp_devrel.testing import eventually_consistent -from gcp_devrel.testing.flaky import flaky import google.api_core.exceptions import google.cloud.bigquery import google.cloud.datastore @@ -24,10 +22,11 @@ import google.cloud.exceptions import google.cloud.pubsub import google.cloud.storage - import pytest + import inspect_content + UNIQUE_STRING = str(uuid.uuid4()).split("-")[0] GCLOUD_PROJECT = os.getenv("GCLOUD_PROJECT") @@ -95,7 +94,8 @@ def subscription_id(topic_id): # Subscribes to a topic. subscriber = google.cloud.pubsub.SubscriberClient() topic_path = subscriber.topic_path(GCLOUD_PROJECT, topic_id) - subscription_path = subscriber.subscription_path(GCLOUD_PROJECT, SUBSCRIPTION_ID) + subscription_path = subscriber.subscription_path( + GCLOUD_PROJECT, SUBSCRIPTION_ID) try: subscriber.create_subscription(subscription_path, topic_path) except google.api_core.exceptions.AlreadyExists: @@ -297,21 +297,21 @@ def test_inspect_gcs_file(bucket, topic_id, subscription_id, capsys): topic_id, subscription_id, ["EMAIL_ADDRESS", "PHONE_NUMBER"], - timeout=420, + timeout=1 ) out, _ = capsys.readouterr() assert "Inspection operation started" in out # Cancel the operation - operation_id = out.split("Inspection operation started: ")[1].split("\n")[0] + operation_id = out.split( + "Inspection operation started: ")[1].split("\n")[0] print(operation_id) client = google.cloud.dlp_v2.DlpServiceClient() client.cancel_dlp_job(operation_id) def test_inspect_gcs_file_with_custom_info_types( - bucket, topic_id, subscription_id, capsys -): + bucket, topic_id, subscription_id, capsys): dictionaries = ["gary@somedomain.com"] regexes = ["\\(\\d{3}\\) \\d{3}-\\d{4}"] @@ -324,20 +324,21 @@ def test_inspect_gcs_file_with_custom_info_types( [], custom_dictionaries=dictionaries, custom_regexes=regexes, - timeout=420, - ) + timeout=1) out, _ = capsys.readouterr() assert "Inspection operation started" in out # Cancel the operation - operation_id = out.split("Inspection operation started: ")[1].split("\n")[0] + operation_id = out.split( + "Inspection operation started: ")[1].split("\n")[0] print(operation_id) client = google.cloud.dlp_v2.DlpServiceClient() client.cancel_dlp_job(operation_id) -def test_inspect_gcs_file_no_results(bucket, topic_id, subscription_id, capsys): +def test_inspect_gcs_file_no_results( + bucket, topic_id, subscription_id, capsys): inspect_content.inspect_gcs_file( GCLOUD_PROJECT, bucket.name, @@ -345,20 +346,19 @@ def test_inspect_gcs_file_no_results(bucket, topic_id, subscription_id, capsys): topic_id, subscription_id, ["EMAIL_ADDRESS", "PHONE_NUMBER"], - timeout=420, - ) + timeout=1) out, _ = capsys.readouterr() assert "Inspection operation started" in out # Cancel the operation - operation_id = out.split("Inspection operation started: ")[1].split("\n")[0] + operation_id = out.split( + "Inspection operation started: ")[1].split("\n")[0] print(operation_id) client = google.cloud.dlp_v2.DlpServiceClient() client.cancel_dlp_job(operation_id) -@pytest.mark.skip(reason="nondeterministically failing") def test_inspect_gcs_image_file(bucket, topic_id, subscription_id, capsys): inspect_content.inspect_gcs_file( GCLOUD_PROJECT, @@ -367,10 +367,16 @@ def test_inspect_gcs_image_file(bucket, topic_id, subscription_id, capsys): topic_id, subscription_id, ["EMAIL_ADDRESS", "PHONE_NUMBER"], - ) + timeout=1) out, _ = capsys.readouterr() - assert "Info type: EMAIL_ADDRESS" in out + assert "Inspection operation started" in out + # Cancel the operation + operation_id = out.split( + "Inspection operation started: ")[1].split("\n")[0] + print(operation_id) + client = google.cloud.dlp_v2.DlpServiceClient() + client.cancel_dlp_job(operation_id) def test_inspect_gcs_multiple_files(bucket, topic_id, subscription_id, capsys): @@ -381,55 +387,62 @@ def test_inspect_gcs_multiple_files(bucket, topic_id, subscription_id, capsys): topic_id, subscription_id, ["EMAIL_ADDRESS", "PHONE_NUMBER"], - ) + timeout=1) out, _ = capsys.readouterr() assert "Inspection operation started" in out # Cancel the operation - operation_id = out.split("Inspection operation started: ")[1].split("\n")[0] + operation_id = out.split( + "Inspection operation started: ")[1].split("\n")[0] print(operation_id) client = google.cloud.dlp_v2.DlpServiceClient() client.cancel_dlp_job(operation_id) -@flaky -def test_inspect_datastore(datastore_project, topic_id, subscription_id, capsys): - @eventually_consistent.call - def _(): - inspect_content.inspect_datastore( - GCLOUD_PROJECT, - datastore_project, - DATASTORE_KIND, - topic_id, - subscription_id, - ["FIRST_NAME", "EMAIL_ADDRESS", "PHONE_NUMBER"], - ) +def test_inspect_datastore( + datastore_project, topic_id, subscription_id, capsys): + inspect_content.inspect_datastore( + GCLOUD_PROJECT, + datastore_project, + DATASTORE_KIND, + topic_id, + subscription_id, + ["FIRST_NAME", "EMAIL_ADDRESS", "PHONE_NUMBER"], + timeout=1) - out, _ = capsys.readouterr() - assert "Info type: EMAIL_ADDRESS" in out + out, _ = capsys.readouterr() + assert "Inspection operation started" in out + # Cancel the operation + operation_id = out.split( + "Inspection operation started: ")[1].split("\n")[0] + print(operation_id) + client = google.cloud.dlp_v2.DlpServiceClient() + client.cancel_dlp_job(operation_id) -@flaky +# @pytest.mark.skip(reason="too slow") def test_inspect_datastore_no_results( - datastore_project, topic_id, subscription_id, capsys -): - @eventually_consistent.call - def _(): - inspect_content.inspect_datastore( - GCLOUD_PROJECT, - datastore_project, - DATASTORE_KIND, - topic_id, - subscription_id, - ["PHONE_NUMBER"], - ) - - out, _ = capsys.readouterr() - assert "No findings" in out - - -@pytest.mark.skip(reason="unknown issue") + datastore_project, topic_id, subscription_id, capsys): + inspect_content.inspect_datastore( + GCLOUD_PROJECT, + datastore_project, + DATASTORE_KIND, + topic_id, + subscription_id, + ["PHONE_NUMBER"], + timeout=1) + + out, _ = capsys.readouterr() + assert "Inspection operation started" in out + # Cancel the operation + operation_id = out.split( + "Inspection operation started: ")[1].split("\n")[0] + print(operation_id) + client = google.cloud.dlp_v2.DlpServiceClient() + client.cancel_dlp_job(operation_id) + + def test_inspect_bigquery(bigquery_project, topic_id, subscription_id, capsys): inspect_content.inspect_bigquery( GCLOUD_PROJECT, @@ -439,7 +452,13 @@ def test_inspect_bigquery(bigquery_project, topic_id, subscription_id, capsys): topic_id, subscription_id, ["FIRST_NAME", "EMAIL_ADDRESS", "PHONE_NUMBER"], - ) + timeout=1) out, _ = capsys.readouterr() - assert "Info type: FIRST_NAME" in out + assert "Inspection operation started" in out + # Cancel the operation + operation_id = out.split( + "Inspection operation started: ")[1].split("\n")[0] + print(operation_id) + client = google.cloud.dlp_v2.DlpServiceClient() + client.cancel_dlp_job(operation_id) diff --git a/dlp/jobs_test.py b/dlp/jobs_test.py index 98acb7464e38..5e53d71b2542 100644 --- a/dlp/jobs_test.py +++ b/dlp/jobs_test.py @@ -13,7 +13,6 @@ # limitations under the License. import os -from flaky import flaky import pytest @@ -66,7 +65,7 @@ def test_list_dlp_jobs(test_job_name, capsys): assert test_job_name not in out -@flaky +@pytest.mark.flaky def test_list_dlp_jobs_with_filter(test_job_name, capsys): jobs.list_dlp_jobs( GCLOUD_PROJECT, diff --git a/dlp/requirements-test.txt b/dlp/requirements-test.txt index d1ad7a9fd107..470977bf2ca8 100644 --- a/dlp/requirements-test.txt +++ b/dlp/requirements-test.txt @@ -1,4 +1,3 @@ pytest==5.3.2 -gcp-devrel-py-tools==0.0.15 flaky==3.6.1 mock==3.0.5 diff --git a/dlp/risk_test.py b/dlp/risk_test.py index 41b514f4da74..5f172bcbc8d2 100644 --- a/dlp/risk_test.py +++ b/dlp/risk_test.py @@ -12,14 +12,12 @@ # See the License for the specific language governing permissions and # limitations under the License. -from flaky import flaky +import os import uuid import google.cloud.pubsub import google.cloud.bigquery - import pytest -import os import risk @@ -160,7 +158,7 @@ def bigquery_project(): bigquery_client.delete_dataset(dataset_ref, delete_contents=True) -@flaky +@pytest.mark.flaky def test_numerical_risk_analysis( topic_id, subscription_id, bigquery_project, capsys ): @@ -178,7 +176,7 @@ def test_numerical_risk_analysis( assert "Value Range:" in out -@flaky +@pytest.mark.flaky def test_categorical_risk_analysis_on_string_field( topic_id, subscription_id, bigquery_project, capsys ): @@ -197,7 +195,7 @@ def test_categorical_risk_analysis_on_string_field( assert "Most common value occurs" in out -@flaky +@pytest.mark.flaky def test_categorical_risk_analysis_on_number_field( topic_id, subscription_id, bigquery_project, capsys ): @@ -215,7 +213,7 @@ def test_categorical_risk_analysis_on_number_field( assert "Most common value occurs" in out -@flaky +@pytest.mark.flaky def test_k_anonymity_analysis_single_field( topic_id, subscription_id, bigquery_project, capsys ): @@ -234,7 +232,7 @@ def test_k_anonymity_analysis_single_field( assert "Class size:" in out -@flaky +@pytest.mark.flaky def test_k_anonymity_analysis_multiple_fields( topic_id, subscription_id, bigquery_project, capsys ): @@ -253,7 +251,7 @@ def test_k_anonymity_analysis_multiple_fields( assert "Class size:" in out -@flaky +@pytest.mark.flaky def test_l_diversity_analysis_single_field( topic_id, subscription_id, bigquery_project, capsys ): @@ -274,7 +272,7 @@ def test_l_diversity_analysis_single_field( assert "Sensitive value" in out -@flaky +@pytest.mark.flaky def test_l_diversity_analysis_multiple_field( topic_id, subscription_id, bigquery_project, capsys ): @@ -295,7 +293,7 @@ def test_l_diversity_analysis_multiple_field( assert "Sensitive value" in out -@flaky +@pytest.mark.flaky def test_k_map_estimate_analysis_single_field( topic_id, subscription_id, bigquery_project, capsys ): @@ -316,7 +314,7 @@ def test_k_map_estimate_analysis_single_field( assert "Values" in out -@flaky +@pytest.mark.flaky def test_k_map_estimate_analysis_multiple_field( topic_id, subscription_id, bigquery_project, capsys ): @@ -337,7 +335,7 @@ def test_k_map_estimate_analysis_multiple_field( assert "Values" in out -@flaky +@pytest.mark.flaky def test_k_map_estimate_analysis_quasi_ids_info_types_equal( topic_id, subscription_id, bigquery_project ):