Skip to content
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.

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

Source Google Search Console: enable high test strictness level in SAT #21503

Merged
merged 5 commits into from
Jan 25, 2023

Conversation

lazebnyi
Copy link
Collaborator

#19044
SAT high strictness

@lazebnyi
Copy link
Collaborator Author

lazebnyi commented Jan 17, 2023

/test connector=connectors/source-google-search-console

🕑 connectors/source-google-search-console https://github.com/airbytehq/airbyte/actions/runs/3943951588
✅ connectors/source-google-search-console https://github.com/airbytehq/airbyte/actions/runs/3943951588
Python tests coverage:

Name                                                            Stmts   Miss  Cover
-----------------------------------------------------------------------------------
source_google_search_console/exceptions.py                          2      0   100%
source_google_search_console/__init__.py                            2      0   100%
source_google_search_console/streams.py                           138      8    94%
source_google_search_console/source.py                             90     10    89%
source_google_search_console/service_account_authenticator.py      14      6    57%
-----------------------------------------------------------------------------------
TOTAL                                                             246     24    90%
	 Name                                                 Stmts   Miss  Cover   Missing
	 ----------------------------------------------------------------------------------
	 source_acceptance_test/base.py                          12      4    67%   16-19
	 source_acceptance_test/config.py                       141      5    96%   87, 93, 239, 243-244
	 source_acceptance_test/conftest.py                     211     95    55%   36, 42-44, 49, 54, 77, 83, 89-91, 110, 115-117, 123-125, 131-132, 137-138, 143, 149, 158-167, 173-178, 193, 217, 248, 254, 262-267, 275-285, 293-306, 311-317, 324-335, 342-358
	 source_acceptance_test/plugin.py                        69     25    64%   22-23, 31, 36, 120-140, 144-148
	 source_acceptance_test/tests/test_core.py              402    115    71%   53, 58, 93-104, 109-116, 120-121, 125-126, 308, 346-363, 376-387, 391-396, 402, 435-440, 478-485, 528-530, 533, 598-606, 618-621, 626, 682-683, 689, 692, 728-738, 751-776
	 source_acceptance_test/tests/test_incremental.py       160     14    91%   58-65, 70-83, 246
	 source_acceptance_test/utils/asserts.py                 39      2    95%   62-63
	 source_acceptance_test/utils/common.py                  94     10    89%   16-17, 32-38, 72, 75
	 source_acceptance_test/utils/compare.py                 62     23    63%   21-51, 68, 97-99
	 source_acceptance_test/utils/connector_runner.py       133     33    75%   24-27, 46-47, 50-54, 57-58, 73-75, 78-80, 83-85, 88-90, 93-95, 124-125, 159-161, 208
	 source_acceptance_test/utils/json_schema_helper.py     107     13    88%   30-31, 38, 41, 65-68, 96, 120, 192-194
	 ----------------------------------------------------------------------------------
	 TOTAL                                                 1609    339    79%

Build Passed

Test summary info:

=========================== short test summary info ============================
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/tests/test_core.py:94: The previous and actual specifications are identical.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/tests/test_core.py:377: The previous and actual discovered catalogs are identical.
=================== 30 passed, 2 skipped in 85.06s (0:01:25) ===================

@evantahler
Copy link
Contributor

evantahler commented Jan 17, 2023

My review is pending the results of this slack converstation. These records look like real ad data for Airbyte, which may be sensitive information.

cc @YowanR

Copy link
Contributor

@evantahler evantahler left a comment

Choose a reason for hiding this comment

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

Rejecting as expected_records contains sensitive data.

I think the path forward here is, sadly, to bypass the stream's acceptance test. It seems like too big of a lift at the moment to spin up a test website and run ads pointing at it...

cc @alafanechere

@lazebnyi
Copy link
Collaborator Author

lazebnyi commented Jan 18, 2023

/test connector=connectors/source-google-search-console

🕑 connectors/source-google-search-console https://github.com/airbytehq/airbyte/actions/runs/3953051816
❌ connectors/source-google-search-console https://github.com/airbytehq/airbyte/actions/runs/3953051816
🐛 https://gradle.com/s/gloejlvowgkmc

Build Failed

Test summary info:

=========================== short test summary info ============================
FAILED test_core.py::TestBasicRead::test_read[inputs0] - Failed: Stream sitem...
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/tests/test_core.py:94: The previous and actual specifications are identical.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/tests/test_core.py:377: The previous and actual discovered catalogs are identical.
============= 1 failed, 29 passed, 2 skipped in 101.82s (0:01:41) ==============

@lazebnyi lazebnyi requested a review from evantahler January 18, 2023 21:24
Copy link
Contributor

@evantahler evantahler left a comment

Choose a reason for hiding this comment

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

{"stream": "search_analytics_by_page", "data": {"clicks": 22, "impressions": 760, "ctr": 0.02894736842105263, "position": 2.375, "site_url": "https://airbyte.io/", "search_type": "web", "date": "2021-09-14", "page": "https://airbyte.io/pricing"}, "emitted_at": 1673994671340}

Still might be protected business data - @Patsy-Airbyte and @YowanR to decide.

@lazebnyi
Copy link
Collaborator Author

/test connector=connectors/source-google-search-console

@lazebnyi lazebnyi requested a review from evantahler January 19, 2023 13:40
@lazebnyi
Copy link
Collaborator Author

lazebnyi commented Jan 19, 2023

/test connector=connectors/source-google-search-console

🕑 connectors/source-google-search-console https://github.com/airbytehq/airbyte/actions/runs/3959089159
❌ connectors/source-google-search-console https://github.com/airbytehq/airbyte/actions/runs/3959089159
🐛 https://gradle.com/s/5ulpijenjly3q

Build Failed

Test summary info:

=========================== short test summary info ============================
FAILED test_core.py::TestBasicRead::test_read[inputs0] - Failed: Stream sitem...
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/tests/test_core.py:94: The previous and actual specifications are identical.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/tests/test_core.py:377: The previous and actual discovered catalogs are identical.
============== 1 failed, 29 passed, 2 skipped in 80.72s (0:01:20) ==============

@lazebnyi
Copy link
Collaborator Author

lazebnyi commented Jan 19, 2023

/test connector=connectors/source-google-search-console

🕑 connectors/source-google-search-console https://github.com/airbytehq/airbyte/actions/runs/3959502256
✅ connectors/source-google-search-console https://github.com/airbytehq/airbyte/actions/runs/3959502256
Python tests coverage:

Name                                                            Stmts   Miss  Cover
-----------------------------------------------------------------------------------
source_google_search_console/exceptions.py                          2      0   100%
source_google_search_console/__init__.py                            2      0   100%
source_google_search_console/streams.py                           138      8    94%
source_google_search_console/source.py                             90     10    89%
source_google_search_console/service_account_authenticator.py      14      6    57%
-----------------------------------------------------------------------------------
TOTAL                                                             246     24    90%
	 Name                                                 Stmts   Miss  Cover   Missing
	 ----------------------------------------------------------------------------------
	 source_acceptance_test/base.py                          12      4    67%   16-19
	 source_acceptance_test/config.py                       141      5    96%   87, 93, 239, 243-244
	 source_acceptance_test/conftest.py                     211     95    55%   36, 42-44, 49, 54, 77, 83, 89-91, 110, 115-117, 123-125, 131-132, 137-138, 143, 149, 158-167, 173-178, 193, 217, 248, 254, 262-267, 275-285, 293-306, 311-317, 324-335, 342-358
	 source_acceptance_test/plugin.py                        69     25    64%   22-23, 31, 36, 120-140, 144-148
	 source_acceptance_test/tests/test_core.py              402    115    71%   53, 58, 93-104, 109-116, 120-121, 125-126, 308, 346-363, 376-387, 391-396, 402, 435-440, 478-485, 528-530, 533, 598-606, 618-621, 626, 682-683, 689, 692, 728-738, 751-776
	 source_acceptance_test/tests/test_incremental.py       160     14    91%   58-65, 70-83, 246
	 source_acceptance_test/utils/asserts.py                 39      2    95%   62-63
	 source_acceptance_test/utils/common.py                  94     10    89%   16-17, 32-38, 72, 75
	 source_acceptance_test/utils/compare.py                 62     23    63%   21-51, 68, 97-99
	 source_acceptance_test/utils/connector_runner.py       133     33    75%   24-27, 46-47, 50-54, 57-58, 73-75, 78-80, 83-85, 88-90, 93-95, 124-125, 159-161, 208
	 source_acceptance_test/utils/json_schema_helper.py     107     13    88%   30-31, 38, 41, 65-68, 96, 120, 192-194
	 ----------------------------------------------------------------------------------
	 TOTAL                                                 1609    339    79%

Build Passed

Test summary info:

=========================== short test summary info ============================
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/tests/test_core.py:94: The previous and actual specifications are identical.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/tests/test_core.py:377: The previous and actual discovered catalogs are identical.
=================== 30 passed, 2 skipped in 98.13s (0:01:38) ===================

@evantahler
Copy link
Contributor

Swapping my review with @YowanR to decide on expected records being secret or not

@evantahler evantahler requested review from YowanR and evantahler and removed request for evantahler January 19, 2023 21:41
@YowanR
Copy link
Contributor

YowanR commented Jan 24, 2023

@lazebnyi I just checked the expected_records. I would consider "clicks": 22, "impressions": 760, to be PIIs. Can we skip those? If not, I suggest that we skip enabling high test strictness for this one. I'm checking this with @Patsy-Airbyte and will get back to you on a final decision today.

@lazebnyi
Copy link
Collaborator Author

lazebnyi commented Jan 24, 2023

/test connector=connectors/source-google-search-console

🕑 connectors/source-google-search-console https://github.com/airbytehq/airbyte/actions/runs/4001201534
✅ connectors/source-google-search-console https://github.com/airbytehq/airbyte/actions/runs/4001201534
Python tests coverage:

Name                                                            Stmts   Miss  Cover
-----------------------------------------------------------------------------------
source_google_search_console/exceptions.py                          2      0   100%
source_google_search_console/__init__.py                            2      0   100%
source_google_search_console/streams.py                           138      8    94%
source_google_search_console/source.py                             90     10    89%
source_google_search_console/service_account_authenticator.py      14      6    57%
-----------------------------------------------------------------------------------
TOTAL                                                             246     24    90%
	 Name                                                 Stmts   Miss  Cover   Missing
	 ----------------------------------------------------------------------------------
	 source_acceptance_test/base.py                          12      4    67%   16-19
	 source_acceptance_test/config.py                       141      5    96%   87, 93, 239, 243-244
	 source_acceptance_test/conftest.py                     211     95    55%   36, 42-44, 49, 54, 77, 83, 89-91, 110, 115-117, 123-125, 131-132, 137-138, 143, 149, 158-167, 173-178, 193, 217, 248, 254, 262-267, 275-285, 293-306, 311-317, 324-335, 342-358
	 source_acceptance_test/plugin.py                        69     25    64%   22-23, 31, 36, 120-140, 144-148
	 source_acceptance_test/tests/test_core.py              402    115    71%   53, 58, 93-104, 109-116, 120-121, 125-126, 308, 346-363, 376-387, 391-396, 402, 435-440, 478-485, 528-530, 533, 598-606, 618-621, 626, 682-683, 689, 692, 728-738, 751-776
	 source_acceptance_test/tests/test_incremental.py       160     14    91%   58-65, 70-83, 246
	 source_acceptance_test/utils/asserts.py                 39      2    95%   62-63
	 source_acceptance_test/utils/common.py                  94     10    89%   16-17, 32-38, 72, 75
	 source_acceptance_test/utils/compare.py                 62     23    63%   21-51, 68, 97-99
	 source_acceptance_test/utils/connector_runner.py       133     33    75%   24-27, 46-47, 50-54, 57-58, 73-75, 78-80, 83-85, 88-90, 93-95, 124-125, 159-161, 208
	 source_acceptance_test/utils/json_schema_helper.py     107     13    88%   30-31, 38, 41, 65-68, 96, 120, 192-194
	 ----------------------------------------------------------------------------------
	 TOTAL                                                 1609    339    79%

Build Passed

Test summary info:

=========================== short test summary info ============================
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/tests/test_core.py:94: The previous and actual specifications are identical.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/tests/test_core.py:377: The previous and actual discovered catalogs are identical.
=================== 30 passed, 2 skipped in 91.61s (0:01:31) ===================

@lazebnyi
Copy link
Collaborator Author

@YowanR From expected records deleted clicks, impressions and all fields connected with country, website urls or type of devices. Waiting your approve to merge PR.

@YowanR
Copy link
Contributor

YowanR commented Jan 25, 2023

Looking at expected_records.jsonl, the data now seems to be free of both PII and business sensitive information which is great! We should be able to proceed with the merge for now. In parallel, we should flag all these connectors to the Unblocker team so that we can look into creating richer samples without PIIs or Business sensitive info.
I'm curious -- what is the impact of reducing the records in expected_records.jsonl? Does it reduce our test coverage?
cc @evantahler

Copy link
Contributor

@YowanR YowanR left a comment

Choose a reason for hiding this comment

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

LGTM!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Source Google Search Console: enable high test strictness level in SAT
4 participants