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

TDL-7455: Add tap-tester test to verify replication of deleted records #168

Merged

Conversation

savan-chovatiya
Copy link
Contributor

Description of change

TDL-7455: add tap-tester test to verify replication of deleted records

  • Added integration test for deleted records

Manual QA steps

Risks

Rollback steps

  • revert this branch

@savan-chovatiya savan-chovatiya changed the base branch from master to crest-master October 11, 2021 04:40
Copy link
Contributor

@kspeer825 kspeer825 left a comment

Choose a reason for hiding this comment

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

Test looks great. I left one suggestion, but approved.

Comment on lines 102 to 105
self.assertTrue('ARCHIVED' not in records_status_sync1)

# Verifying that ARCHIVED records are returned for sync 2
self.assertTrue('ARCHIVED' in records_status_sync2)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
self.assertTrue('ARCHIVED' not in records_status_sync1)
# Verifying that ARCHIVED records are returned for sync 2
self.assertTrue('ARCHIVED' in records_status_sync2)
self.assertNotIn('ARCHIVED', records_status_sync1)
# Verifying that ARCHIVED records are returned for sync 2
self.assertIn('ARCHIVED', records_status_sync2)

Copy link
Contributor

Choose a reason for hiding this comment

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

I find that assertTrue throws a harder-to-debug AssertionError. Something like False != True as opposed to 'ARCHIVED not in records_status_sync2 when using assertIn.

@hpatel41 hpatel41 merged commit 12bb05d into crest-master Nov 19, 2021
@savan-chovatiya savan-chovatiya mentioned this pull request Nov 19, 2021
KrisPersonal pushed a commit that referenced this pull request Nov 19, 2021
* TDL-9728: Stream `ads_insights_age_gender` has unexpected datatype for replication key field `date_start` (#172)

* added format as date-time in schema file

* added code coverage

* added check for date format in the bookmark test

* added the check for first sync messages

Co-authored-by: namrata270998 <namrata.brahmbhatt@crestdatasys.com>

* TDL-9809: `forced-replication-method` missing from metadata for some streams and TDL-9872: replication keys are not specified as expected in discoverable metadata	 (#167)

* added valid replication keys in catalog

* modified the code

* TDL-9809: Added replication keys in metadata

* adde code coverage

* Resolved review comments

Co-authored-by: harshpatel4_crest <harsh.patel4@crestdatasys.com>
Co-authored-by: namrata270998 <namrata.brahmbhatt@crestdatasys.com>

* TDL-7455: Add tap-tester test to verify replication of deleted records	 (#168)

* TDL-7455: Added archived data integration test

* TDL-7455: Updated integration test

* added code coverage

* Resolved review comment

Co-authored-by: namrata270998 <namrata.brahmbhatt@crestdatasys.com>

* TDL-7596: Add tap-tester test for attribution window (#169)

* added tap tester test for attribution window

* updated the code

* added code coverage

* updated the code according to the comments

* updated code to raise error when attribution window is not 1, 7, 28

* test: run invalid attribution window intergation test

* updated test case

* test: updated test case code

* test: test invalid attribution window

* test: test invalid attribution window

* test: test invalid attribution window

* test: test invalid attribution window

* test: run invalid attribution window test case

* added intergation test for invalid sttribution window

Co-authored-by: namrata270998 <namrata.brahmbhatt@crestdatasys.com>

Co-authored-by: namrata270998 <namrata.brahmbhatt@crestdatasys.com>
Co-authored-by: savan-chovatiya <80703490+savan-chovatiya@users.noreply.github.com>
KrisPersonal added a commit that referenced this pull request Nov 23, 2021
…ve requests that are not retrying (#171)

* TDL-6148: Added retry for Attribute error of sync batches

* TDL-6148: Removed unused imports from unit tests

* TDL-13267: Added retry for 500 error of AdCreatives

* TDL-6148: Add AttributeError backoff for all sync functions

* added code coverage

* Resolved review comment

* Resolved review comments

* Added code comments

* Resolved review comment

* TDL-9728: Stream `ads_insights_age_gender` has unexpected datatype for replication key field `date_start` (#172)

* added format as date-time in schema file

* added code coverage

* added check for date format in the bookmark test

* added the check for first sync messages

Co-authored-by: namrata270998 <namrata.brahmbhatt@crestdatasys.com>

* TDL-9809: `forced-replication-method` missing from metadata for some streams and TDL-9872: replication keys are not specified as expected in discoverable metadata	 (#167)

* added valid replication keys in catalog

* modified the code

* TDL-9809: Added replication keys in metadata

* adde code coverage

* Resolved review comments

Co-authored-by: harshpatel4_crest <harsh.patel4@crestdatasys.com>
Co-authored-by: namrata270998 <namrata.brahmbhatt@crestdatasys.com>

* TDL-7455: Add tap-tester test to verify replication of deleted records	 (#168)

* TDL-7455: Added archived data integration test

* TDL-7455: Updated integration test

* added code coverage

* Resolved review comment

Co-authored-by: namrata270998 <namrata.brahmbhatt@crestdatasys.com>

Co-authored-by: namrata270998 <namrata.brahmbhatt@crestdatasys.com>
Co-authored-by: Harsh <80324346+harshpatel4crest@users.noreply.github.com>
Co-authored-by: harshpatel4_crest <harsh.patel4@crestdatasys.com>
Co-authored-by: KrisPersonal <66801357+KrisPersonal@users.noreply.github.com>
KrisPersonal added a commit that referenced this pull request Dec 14, 2021
* TDL-15863: Added request timeout and unit tests

* TDL-15863: Added unit tests

* TDL-15863: Removed commented code

* TDL-15863: Added request_timeout lookup from config also

* added code coverage

* TDL-15863: Added float type cast for timeout

* Updated comment

* added code change for empty string timeout value from config

* Added empty string handling in request_timeout param

* Resolved review comment

* Added if else for request timeout

* Added backoff for ConnectionError

* TDL-9728: Stream `ads_insights_age_gender` has unexpected datatype for replication key field `date_start` (#172)

* added format as date-time in schema file

* added code coverage

* added check for date format in the bookmark test

* added the check for first sync messages

Co-authored-by: namrata270998 <namrata.brahmbhatt@crestdatasys.com>

* TDL-9809: `forced-replication-method` missing from metadata for some streams and TDL-9872: replication keys are not specified as expected in discoverable metadata	 (#167)

* added valid replication keys in catalog

* modified the code

* TDL-9809: Added replication keys in metadata

* adde code coverage

* Resolved review comments

Co-authored-by: harshpatel4_crest <harsh.patel4@crestdatasys.com>
Co-authored-by: namrata270998 <namrata.brahmbhatt@crestdatasys.com>

* TDL-7455: Add tap-tester test to verify replication of deleted records	 (#168)

* TDL-7455: Added archived data integration test

* TDL-7455: Updated integration test

* added code coverage

* Resolved review comment

Co-authored-by: namrata270998 <namrata.brahmbhatt@crestdatasys.com>

Co-authored-by: namrata270998 <namrata.brahmbhatt@crestdatasys.com>
Co-authored-by: harshpatel4_crest <harsh.patel4@crestdatasys.com>
Co-authored-by: Harsh <80324346+harshpatel4crest@users.noreply.github.com>
Co-authored-by: KrisPersonal <66801357+KrisPersonal@users.noreply.github.com>
jesuejunior pushed a commit to sixcodes/tap-facebook that referenced this pull request Mar 17, 2023
* TDL-9728: Stream `ads_insights_age_gender` has unexpected datatype for replication key field `date_start` (singer-io#172)

* added format as date-time in schema file

* added code coverage

* added check for date format in the bookmark test

* added the check for first sync messages

Co-authored-by: namrata270998 <namrata.brahmbhatt@crestdatasys.com>

* TDL-9809: `forced-replication-method` missing from metadata for some streams and TDL-9872: replication keys are not specified as expected in discoverable metadata	 (singer-io#167)

* added valid replication keys in catalog

* modified the code

* TDL-9809: Added replication keys in metadata

* adde code coverage

* Resolved review comments

Co-authored-by: harshpatel4_crest <harsh.patel4@crestdatasys.com>
Co-authored-by: namrata270998 <namrata.brahmbhatt@crestdatasys.com>

* TDL-7455: Add tap-tester test to verify replication of deleted records	 (singer-io#168)

* TDL-7455: Added archived data integration test

* TDL-7455: Updated integration test

* added code coverage

* Resolved review comment

Co-authored-by: namrata270998 <namrata.brahmbhatt@crestdatasys.com>

* TDL-7596: Add tap-tester test for attribution window (singer-io#169)

* added tap tester test for attribution window

* updated the code

* added code coverage

* updated the code according to the comments

* updated code to raise error when attribution window is not 1, 7, 28

* test: run invalid attribution window intergation test

* updated test case

* test: updated test case code

* test: test invalid attribution window

* test: test invalid attribution window

* test: test invalid attribution window

* test: test invalid attribution window

* test: run invalid attribution window test case

* added intergation test for invalid sttribution window

Co-authored-by: namrata270998 <namrata.brahmbhatt@crestdatasys.com>

Co-authored-by: namrata270998 <namrata.brahmbhatt@crestdatasys.com>
Co-authored-by: savan-chovatiya <80703490+savan-chovatiya@users.noreply.github.com>
jesuejunior pushed a commit to sixcodes/tap-facebook that referenced this pull request Mar 17, 2023
…ve requests that are not retrying (singer-io#171)

* TDL-6148: Added retry for Attribute error of sync batches

* TDL-6148: Removed unused imports from unit tests

* TDL-13267: Added retry for 500 error of AdCreatives

* TDL-6148: Add AttributeError backoff for all sync functions

* added code coverage

* Resolved review comment

* Resolved review comments

* Added code comments

* Resolved review comment

* TDL-9728: Stream `ads_insights_age_gender` has unexpected datatype for replication key field `date_start` (singer-io#172)

* added format as date-time in schema file

* added code coverage

* added check for date format in the bookmark test

* added the check for first sync messages

Co-authored-by: namrata270998 <namrata.brahmbhatt@crestdatasys.com>

* TDL-9809: `forced-replication-method` missing from metadata for some streams and TDL-9872: replication keys are not specified as expected in discoverable metadata	 (singer-io#167)

* added valid replication keys in catalog

* modified the code

* TDL-9809: Added replication keys in metadata

* adde code coverage

* Resolved review comments

Co-authored-by: harshpatel4_crest <harsh.patel4@crestdatasys.com>
Co-authored-by: namrata270998 <namrata.brahmbhatt@crestdatasys.com>

* TDL-7455: Add tap-tester test to verify replication of deleted records	 (singer-io#168)

* TDL-7455: Added archived data integration test

* TDL-7455: Updated integration test

* added code coverage

* Resolved review comment

Co-authored-by: namrata270998 <namrata.brahmbhatt@crestdatasys.com>

Co-authored-by: namrata270998 <namrata.brahmbhatt@crestdatasys.com>
Co-authored-by: Harsh <80324346+harshpatel4crest@users.noreply.github.com>
Co-authored-by: harshpatel4_crest <harsh.patel4@crestdatasys.com>
Co-authored-by: KrisPersonal <66801357+KrisPersonal@users.noreply.github.com>
jesuejunior pushed a commit to sixcodes/tap-facebook that referenced this pull request Mar 17, 2023
* TDL-15863: Added request timeout and unit tests

* TDL-15863: Added unit tests

* TDL-15863: Removed commented code

* TDL-15863: Added request_timeout lookup from config also

* added code coverage

* TDL-15863: Added float type cast for timeout

* Updated comment

* added code change for empty string timeout value from config

* Added empty string handling in request_timeout param

* Resolved review comment

* Added if else for request timeout

* Added backoff for ConnectionError

* TDL-9728: Stream `ads_insights_age_gender` has unexpected datatype for replication key field `date_start` (singer-io#172)

* added format as date-time in schema file

* added code coverage

* added check for date format in the bookmark test

* added the check for first sync messages

Co-authored-by: namrata270998 <namrata.brahmbhatt@crestdatasys.com>

* TDL-9809: `forced-replication-method` missing from metadata for some streams and TDL-9872: replication keys are not specified as expected in discoverable metadata	 (singer-io#167)

* added valid replication keys in catalog

* modified the code

* TDL-9809: Added replication keys in metadata

* adde code coverage

* Resolved review comments

Co-authored-by: harshpatel4_crest <harsh.patel4@crestdatasys.com>
Co-authored-by: namrata270998 <namrata.brahmbhatt@crestdatasys.com>

* TDL-7455: Add tap-tester test to verify replication of deleted records	 (singer-io#168)

* TDL-7455: Added archived data integration test

* TDL-7455: Updated integration test

* added code coverage

* Resolved review comment

Co-authored-by: namrata270998 <namrata.brahmbhatt@crestdatasys.com>

Co-authored-by: namrata270998 <namrata.brahmbhatt@crestdatasys.com>
Co-authored-by: harshpatel4_crest <harsh.patel4@crestdatasys.com>
Co-authored-by: Harsh <80324346+harshpatel4crest@users.noreply.github.com>
Co-authored-by: KrisPersonal <66801357+KrisPersonal@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants