-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Extend abnormal state test to include cursor format check #39136
Extend abnormal state test to include cursor format check #39136
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
8df2adc
to
7155c3b
Compare
@roman-yermilov-gl Can we have a test using some of the critical sources, like:
We can test the |
I have already run tests on all connectors, including the critical ones. This test is not failing. |
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.
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.
@roman-yermilov-gl There are multiple regressions with no straightforward way to identify the issue, based on the latest Here is the proposed solution, to modify this statement, instead of empty types check for # 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? |
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