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

Implement table_reset test in the new framework #161

Merged
merged 10 commits into from
Jul 12, 2023
Merged

Implement table_reset test in the new framework #161

merged 10 commits into from
Jul 12, 2023

Conversation

bhuvana-talend
Copy link
Contributor

Description of change

https://jira.talendforge.org/browse/TDL-22960

  • Rewrote this test from test_salesforce_incremental_table_reset.py to use the new framework.
  • It also provides the opportunity to test the new testcase table_reset_test.py in the new framework

QA steps

  • automated tests passing
  • manual qa steps passing (list below)

Risks

Rollback steps

  • revert this branch

class SFTableResetTest(TableResetTest, SFBaseTest):
"""tap-salesforce Table reset test implementation"""

salesforce_api = 'BULK'
Copy link
Contributor

Choose a reason for hiding this comment

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

This should probably go in SFBaseTest since it's used by SFBaseTest.get_properties

Choose a reason for hiding this comment

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

We should probably set a default in the base case, which can be overriden by setting the property here if needed. Also, do we need to run this test for both BULK and REST? We could import TableResetTest again in another module and class and set it in that test to REST.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved this to sfbase.py


salesforce_api = 'BULK'
reset_stream = 'User'
replication_method = "INCREMENTAL"
Copy link
Contributor

Choose a reason for hiding this comment

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

See my comment in the framework about this

Choose a reason for hiding this comment

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

This already exists in the tap-tester base_case. No need to define it again here.

    INCREMENTAL = "INCREMENTAL"  # replication method value in metadata
    FULL_TABLE = "FULL_TABLE"  # replication method value in metadata

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed this

Comment on lines 19 to 28
@staticmethod
def convert_state_to_utc(date_str):
"""
Convert a saved bookmark value of form '2020-08-25T13:17:36-07:00' to
a string formatted utc datetime,
in order to compare aginast json formatted datetime values
"""
date_object = dateutil.parser.parse(date_str)
date_object_utc = date_object.astimezone(tz=pytz.UTC)
return datetime.datetime.strftime(date_object_utc, "%Y-%m-%dT%H:%M:%SZ")
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't seem like a salesforce-table-reset specific function. Should we move this into the framework?

Choose a reason for hiding this comment

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

This is similar to parse_date and timedelta_formatted. We should put this in base case in the same section or see if we can enhance that functionality to also convert to UTC.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

moved this to framework base_case.py
This method in turn calls parse_date and then converts to utc

tests/sfbase.py Outdated
Comment on lines 879 to 956
@staticmethod
def get_unsupported_by_rest_api():
"""The streams listed here are not supported by the REST API"""
unsupported_streams = {
'Announcement',
'CollaborationGroupRecord',
'ContentDocumentLink',
'ContentFolderMember',
'DataStatistics',
'EntityParticle',
'FieldDefinition',
'FlexQueueItem',
'IdeaComment',
'OwnerChangeOptionInfo',
'PicklistValueInfo',
'PlatformAction',
'RelationshipDomain',
'RelationshipInfo',
'SearchLayout',
'SiteDetail',
'UserEntityAccess',
'UserFieldAccess',
'Vote',
'RecordActionHistory',
'FlowVersionView',
'FlowVariableView',
'AppTabMember',
'ColorDefinition',
'IconDefinition',
}

return unsupported_streams

def get_unsupported_by_bulk_api(self):
unsupported_streams_rest = self.get_unsupported_by_rest_api()
unsupported_streams_bulk_only= {
'AcceptedEventRelation',
'AssetTokenEvent',
'AttachedContentNote',
'CaseStatus',
'ContentFolderItem',
'ContractStatus',
'DeclinedEventRelation',
'EventWhoRelation',
'PartnerRole',
'QuoteTemplateRichTextData',
'RecentlyViewed',
'SolutionStatus',
'TaskPriority',
'TaskWhoRelation',
'TaskStatus',
'UndecidedEventRelation',
'OrderStatus',
'WorkOrderStatus',
'WorkOrderLineItemStatus',
'ServiceAppointmentStatus',
'ServiceAppointmentStatus',
'FieldSecurityClassification',
# BUG_TODO | the following streams are undocumented
'WorkStepStatus',
'ShiftStatus',
}

return unsupported_streams_bulk_only | unsupported_streams_rest

def is_unsupported_by_rest_api(self, stream):
"""returns True if stream is unsupported by REST API"""

return stream in self.get_unsupported_by_rest_api()

def is_unsupported_by_bulk_api(self, stream):
"""
returns True if stream is unsupported by BULK API

BULK API does not support any streams that are unsupported by the REST API and
in addition does not support the streams listed below.
"""
return stream in self.get_unsupported_by_bulk_api()

Choose a reason for hiding this comment

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

I thought we had discussed in slack that we were going to remove this as it isn't necessary yet and we have it in the other base and then we can add it in if needed when a test requires it as we transition to the new framework.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, missed it, Removed this method now

Copy link

@HarrisonMarcRose HarrisonMarcRose left a comment

Choose a reason for hiding this comment

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

left a comment

tests/sfbase.py Outdated
Comment on lines 880 to 942
def get_unsupported_by_rest_api():
"""The streams listed here are not supported by the REST API"""
unsupported_streams = {
'Announcement',
'CollaborationGroupRecord',
'ContentDocumentLink',
'ContentFolderMember',
'DataStatistics',
'EntityParticle',
'FieldDefinition',
'FlexQueueItem',
'IdeaComment',
'OwnerChangeOptionInfo',
'PicklistValueInfo',
'PlatformAction',
'RelationshipDomain',
'RelationshipInfo',
'SearchLayout',
'SiteDetail',
'UserEntityAccess',
'UserFieldAccess',
'Vote',
'RecordActionHistory',
'FlowVersionView',
'FlowVariableView',
'AppTabMember',
'ColorDefinition',
'IconDefinition',
}

return unsupported_streams

def get_unsupported_by_bulk_api(self):
unsupported_streams_rest = self.get_unsupported_by_rest_api()
unsupported_streams_bulk_only= {
'AcceptedEventRelation',
'AssetTokenEvent',
'AttachedContentNote',
'CaseStatus',
'ContentFolderItem',
'ContractStatus',
'DeclinedEventRelation',
'EventWhoRelation',
'PartnerRole',
'QuoteTemplateRichTextData',
'RecentlyViewed',
'SolutionStatus',
'TaskPriority',
'TaskWhoRelation',
'TaskStatus',
'UndecidedEventRelation',
'OrderStatus',
'WorkOrderStatus',
'WorkOrderLineItemStatus',
'ServiceAppointmentStatus',
'ServiceAppointmentStatus',
'FieldSecurityClassification',
# BUG_TODO | the following streams are undocumented
'WorkStepStatus',
'ShiftStatus',
}

return unsupported_streams_bulk_only | unsupported_streams_rest

Choose a reason for hiding this comment

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

If we aren't using these methods in our new test (which it looks like we aren't) we should remove this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed these unused methods

"""tap-salesforce Table reset test implementation
Currently tests only the stream with Incremental replication method"""

#reset_stream = 'User'

Choose a reason for hiding this comment

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

Don't need this since we have it below. Although, if you make this a property then this would work and you could remove the other one. I think that looks better but either way works.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Made it as a property

@bhtowles bhtowles merged commit 5d46c8e into master Jul 12, 2023
2 checks passed
@bhtowles bhtowles deleted the tdl-22960 branch July 12, 2023 13:19
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.

4 participants