Extend abnormal state test to include cursor format check

[roman-yermilov-gl]

What

https://github.com/airbytehq/airbyte-internal-issues/issues/7940

How

Extended the test to include checks of the given future state format and the output state format

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment

Name	Status	Preview	Comments	Updated (UTC)
airbyte-docs	⬜️ Ignored (Inspect)	Visit Preview		Jun 7, 2024 0:01am

[bazarnov]

@roman-yermilov-gl Can we have a test using some of the critical sources, like:

• Facebook-marketing
• Shopify
• Salesforce
• Hubspot

We can test the incremental part only, to save time and rate-limits. We need to be sure we don't break anything. Poke me if you need any help on that. Thanks.

[roman-yermilov-gl]

@roman-yermilov-gl Can we have a test using some of the critical sources, like:

• Facebook-marketing
• Shopify
• Salesforce
• Hubspot

We can test the incremental part only, to save time and rate-limits. We need to be sure we don't break anything. Poke me if you need any help on that. Thanks.

I have already run tests on all connectors, including the critical ones. This test is not failing.

[natikgadzhi]

Looks good, and it's CAT change in the end of the day, so I'm ok with merging. If there are additional comments and feedback, @roman-yermilov-gl please follow up with them when you're back. But, if we need this to progress forward, I'm ok.

[bazarnov]

@roman-yermilov-gl There are multiple regressions with no straightforward way to identify the issue, based on the latest health-report.
This is the source-shopify example of the regression:

Here is the proposed solution, to modify this statement, instead of empty types check for all(<future_state_values>):

# collect missing future_state
missing_future_state_cursor_values_per_stream = {
    stream_name: future_state 
    for stream_name, future_state in future_state_cursor_values_per_stream.items()
    if not future_state
}
                    
assert not missing_future_state_cursor_values_per_stream, "Future state must be set up for all given streams. Streams without the `future_state`:\n\n"

This will narrow down the errors to the specific ones, instead of the general error message.

After this change such output is expected, if there are errors for this test:

E       AssertionError: Future state must be set up for all given streams. Streams without the `future_state`:
E         
E         
E       assert not {'blogs': []}

Does this make sense to be patched, real quick?

cc @lazebnyi @oustynova @natikgadzhi