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

[BUG] ParquetOutputCodecTest may not be testing correctly #3992

Closed
dlvenable opened this issue Jan 19, 2024 · 2 comments · Fixed by #4698
Closed

[BUG] ParquetOutputCodecTest may not be testing correctly #3992

dlvenable opened this issue Jan 19, 2024 · 2 comments · Fixed by #4698
Assignees
Labels
bug Something isn't working maintenance Issues to help maintain the project, such as improving builds, testing, etc.

Comments

@dlvenable
Copy link
Member

dlvenable commented Jan 19, 2024

Describe the bug

I removed some try-catch lines in the ParquetOutputCodecTest test. These tests began to fail.

Something is definitely wrong as the lines in question are part of the setup. It may be that the tests are not actually testing what they claim to test.

To Reproduce

Remove these lines:

} catch (Exception parquetException) {
LOG.error("Failed to parse Parquet", parquetException);
}

} catch (Exception parquetException) {
LOG.error("Failed to parse Parquet", parquetException);

Run the tests.

Quite a few of them fail.

Expected behavior

I should be able to run tests without this code throwing exceptions at all.

Additional context

This was found while removing ex.printStackTrace() lines:

https://github.com/opensearch-project/data-prepper/pull/3991/files#r1459559030

@dlvenable dlvenable added bug Something isn't working untriaged labels Jan 19, 2024
@dlvenable dlvenable added maintenance Issues to help maintain the project, such as improving builds, testing, etc. and removed untriaged labels Jan 23, 2024
@dlvenable dlvenable added this to the v2.8 milestone Jan 23, 2024
@dlvenable dlvenable removed this from the v2.8 milestone Apr 16, 2024
@dlvenable dlvenable self-assigned this Apr 16, 2024
@dlvenable
Copy link
Member Author

Also, we found that some of the assertions are loops. However, the size of these loops is not asserted. Thus, these assertions are not even happening.

@dlvenable
Copy link
Member Author

Also, @kkondaka found this:

List<Map<String, Object>> actualRecords = createParquetRecordsList(new ByteArrayInputStream(tempFile.toString().getBytes()));

The line above is not actually loading the file. It gets the path as bytes.

It should look like:

List<Map<String, Object>> actualRecords = createParquetRecordsList(new FileInputStream(tempFile));

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working maintenance Issues to help maintain the project, such as improving builds, testing, etc.
Projects
Development

Successfully merging a pull request may close this issue.

1 participant