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

Fix BufferedStreamConsumer tests #7773

Merged
merged 5 commits into from
Nov 9, 2021

Conversation

sherifnada
Copy link
Contributor

What

#7719 introduced changes to bufferedStreamconsumer but I goofed and forgot to fix unit tests. This PR fixes the unit tests by making them byte-based instead of record-count-based.

@sherifnada sherifnada temporarily deployed to more-secrets November 9, 2021 05:26 Inactive
- "format"
- "azure_blob_storage_account_name"
- "azure_blob_storage_account_key"
- "format"
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these formatting changes expected?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this was the output of the format


@Test
public void testIt() {
for (int i = 1; i < 1000; i++) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Iterating 1000 times does not seem necessary since the current implementation is a naive one.

@sherifnada sherifnada temporarily deployed to more-secrets November 9, 2021 05:52 Inactive
Copy link
Contributor

@ChristopheDuong ChristopheDuong left a comment

Choose a reason for hiding this comment

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

This should fix master build once it passes :airbyte-integrations:bases:base-java:test?

- "format"
- "azure_blob_storage_account_name"
- "azure_blob_storage_account_key"
- "format"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this was the output of the format

@sherifnada sherifnada merged commit 62992bf into master Nov 9, 2021
@sherifnada sherifnada deleted the sherif/fix-bufferedstreamconsumer-tests branch November 9, 2021 20:50
@sherifnada sherifnada temporarily deployed to more-secrets November 9, 2021 20:51 Inactive
schlattk pushed a commit to schlattk/airbyte that referenced this pull request Jan 4, 2022
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.

None yet

3 participants