-
Notifications
You must be signed in to change notification settings - Fork 23
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-20215: Add missing integration tests #42
base: crest-master
Are you sure you want to change the base?
TDL-20215: Add missing integration tests #42
Conversation
tests/test_sftp_all_fields.py
Outdated
expected_all_keys = stream_to_all_catalog_fields[stream] | ||
|
||
# Tap is not writing _sdc_extra so removing it from expected keys. | ||
expected_all_keys = expected_all_keys - {"_sdc_extra"} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can get _sdc_extra
if we override generate_simple_csv_lines_typeA
and send extra field data for one or two rows.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overrided generate_simple_csv_lines_typeA() function, and generated data for sdc_extra field.
tests/test_sftp_automatic.py
Outdated
def name(self): | ||
return "tap_tester_sftp_automatic_fields" | ||
|
||
def get_files(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems duplicating can we move this to base.py?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, Moved get_files() to base for the most used common structure, and override in other files if requires.
} | ||
] | ||
|
||
def setUp(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add function comments for each file's function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added function comments
tests/test_sftp_automatic.py
Outdated
] | ||
|
||
def setUp(self): | ||
if not all([x for x in [os.getenv('TAP_SFTP_USERNAME'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Setup is also used in many files so move this to base.py and for diff lines, use super and add those lines.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The SetUp() function is related to pytest so it will require in each file to set up something before running that particular test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes looks good but build is failing may be need to update some stuffs at CCI side so can you please create a card for the same and do?
Description of change
TDL-20215: Add missing integration tests
Manual QA steps
Risks
Rollback steps