Source Bing Ads: add integration tests #35961
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
octavia-squidington-iii
added
connectors/source/bing-ads
area/connectors
There was a problem hiding this comment.
Conditional approval on improving the maintainability of
- airbyte-integrations/connectors/source-bing-ads/unit_tests/integrations/test_app_install_ad_labels_stream.py
- airbyte-integrations/connectors/source-bing-ads/unit_tests/integrations/test_budget_stream.py
| self.auth_client(http_mocker) | ||
| output, _ = self.read_stream(self.stream_name, SyncMode.full_refresh, self._config, "app_install_ad_labels_empty") | ||
| assert len(output.records) == 0 | ||
| assert len(output.logs) == 10 |
There was a problem hiding this comment.
This seems rather flaky i.e. if someone add a log, this will fail. Why are logs important here in terms of behavior? Can we scope the test to that behavior?
There was a problem hiding this comment.
we discussed this in previous pr #35201 (comment)
...ions/connectors/source-bing-ads/unit_tests/integrations/test_app_install_ad_labels_stream.py
Outdated
Show resolved
Hide resolved
| "app_install_ad_labels_with_state", | ||
| state | ||
| ) | ||
| assert len(output.records) == 4 |
There was a problem hiding this comment.
Should we have less records in this test than in test_incremental_read_cursor_value_matches_value_from_most_recent_record since we start from a state? It seems like in integration test, the outcome is just that gets DownloadParams object part so I think I would remove this assertion
There was a problem hiding this comment.
we don't filter records in this streams, so number of records depends of response, I will remove this assert
| @@ -0,0 +1,54 @@ | |||
| # Copyright (c) 2023 Airbyte, Inc., all rights reserved. | |||
There was a problem hiding this comment.
The same comments in airbyte-integrations/connectors/source-bing-ads/unit_tests/integrations/test_app_install_ad_labels_stream.py apply here
airbyte-integrations/connectors/source-bing-ads/unit_tests/integrations/test_budget_stream.py
Outdated
Show resolved
Hide resolved
resolved: https://github.com/airbytehq/airbyte-internal-issues/issues/6654
Added integration tests for streams:
1. app_install_ads
2. app_install_ad_labels
3. age_gender_audience_report_hourly
4. account_impression_performance_report_hourly
5. keyword_performance_report_hourly
6. ad_performance_report_hourly
7. ad_group_impression_performance_report_hourly
8. campaign_performance_report_hourly
9. campaign_impression_performance_report_hourly
10. geographic_performance_report_hourly
11. search_query_performance_report_hourly
12. user_location_performance_report_hourly
13. ad_group_performance_report_hourly
14. account_performance_report_hourly
*_hourly_reports cannot be tested with cat as sync is too long. app_install_ads and app_install_ad_labels streams are empty.
Also updated expected records with *monthly reports and removed them from cat cinfig.